NXP-8733: do not allow special characters in command line parameters.…

… No need to quote parameters in the 'parameterString' of the contributions
Gagnavarslan · Feb 22, 2012 · 2c94034 · 2c94034
1 parent cf7c6f9
commit 2c94034
Show file tree

Hide file tree

Showing 6 changed files with 61 additions and 28 deletions.
diff --git a/...executor/src/main/java/org/nuxeo/ecm/platform/commandline/executor/api/CmdParameters.java b/...executor/src/main/java/org/nuxeo/ecm/platform/commandline/executor/api/CmdParameters.java
@@ -46,7 +46,7 @@ public void addNamedParameter(String name, String value) {
     }
 
     public void addNamedParameter(String name, File file) {
-        addNamedParameter(name, "\"" + file.getAbsolutePath() + "\"");
+        addNamedParameter(name, file.getAbsolutePath());
     }
 
     public Map<String, String> getParameters() {

diff --git a/.../java/org/nuxeo/ecm/platform/commandline/executor/service/executors/AbstractExecutor.java b/.../java/org/nuxeo/ecm/platform/commandline/executor/service/executors/AbstractExecutor.java
@@ -22,6 +22,7 @@
 import java.util.ArrayList;
 import java.util.List;
 import java.util.Map;
+import java.util.regex.Pattern;
 
 import org.nuxeo.ecm.platform.commandline.executor.api.CmdParameters;
 import org.nuxeo.ecm.platform.commandline.executor.service.CommandLineDescriptor;
@@ -33,14 +34,16 @@
  */
 public abstract class AbstractExecutor {
 
+    private static final Pattern VALID_PARAMETER_PATTERN = Pattern.compile("[a-zA-Z_0-9-.%:/\\\\ ]+");
+
     public static boolean isWindows() {
         String osName = System.getProperty("os.name");
         return osName.toLowerCase().contains("windows");
     }
 
     /**
-     * Returns parameters as a String after having replaced parameterized
-     * values inside.
+     * Returns parameters as a String after having replaced parameterized values
+     * inside.
      *
      * @param cmdDesc CommandLineDescriptor containing parameters
      * @param params parameterized values
@@ -77,10 +80,19 @@ public static String[] getParametersArray(CommandLineDescriptor cmdDesc,
     private static String replaceParams(Map<String, String> paramsValues,
             String paramString) {
         for (String pname : paramsValues.keySet()) {
-            paramString = paramString.replace("#{" + pname + "}",
-                    paramsValues.get(pname));
+            String param = "#{" + pname + "}";
+            if (paramString.contains(param)) {
+                String value = paramsValues.get(pname);
+                if (!VALID_PARAMETER_PATTERN.matcher(value).matches()) {
+                    throw new IllegalArgumentException(
+                            String.format(
+                                    "'%s' contains illegal characters. It should match: %s",
+                                    value, VALID_PARAMETER_PATTERN));
+                }
+                paramString = paramString.replace("#{" + pname + "}",
+                        String.format("\"%s\"", value));
+            }
         }
         return paramString;
     }
-
 }
diff --git a/...xecutor/src/test/java/org/nuxeo/ecm/platform/commandline/executor/tests/TestCommands.java b/...xecutor/src/test/java/org/nuxeo/ecm/platform/commandline/executor/tests/TestCommands.java
@@ -72,23 +72,23 @@ public void testCmdParamatersParsing() throws Exception {
         String parsedParamString = AbstractExecutor.getParametersString(
                 cmdDesc, params);
         assertEquals(
-                "-a --lang=fr_FR --encoding=utf-8 -H --rem-sgml-check=alt < /tmp/textMe.txt",
+                "-a --lang=\"fr_FR\" --encoding=\"utf-8\" -H --rem-sgml-check=alt < \"/tmp/textMe.txt\"",
                 parsedParamString);
 
         // test with File param
         params.addNamedParameter("textFile", textFile);
         parsedParamString = AbstractExecutor.getParametersString(cmdDesc,
                 params);
         // System.out.println("command:" + parsedParamString);
-        assertTrue(parsedParamString.startsWith("-a --lang=fr_FR --encoding=utf-8 -H --rem-sgml-check=alt < "));
+        assertTrue(parsedParamString.startsWith("-a --lang=\"fr_FR\" --encoding=\"utf-8\" -H --rem-sgml-check=alt < "));
         assertTrue(parsedParamString.contains(System.getProperties().getProperty(
                 "java.io.tmpdir")));
 
         String[] res = AbstractExecutor.getParametersArray(cmdDesc, params);
         assertEquals(7, res.length);
         assertEquals("-a", res[0]);
-        assertEquals("--lang=fr_FR", res[1]);
-        assertEquals("--encoding=utf-8", res[2]);
+        assertEquals("--lang=\"fr_FR\"", res[1]);
+        assertEquals("--encoding=\"utf-8\"", res[2]);
         assertEquals("-H", res[3]);
         assertEquals("--rem-sgml-check=alt", res[4]);
         assertEquals("<", res[5]);

diff --git a/...nvert/src/main/java/org/nuxeo/ecm/platform/convert/plugins/CommandLineBasedConverter.java b/...nvert/src/main/java/org/nuxeo/ecm/platform/convert/plugins/CommandLineBasedConverter.java
@@ -26,6 +26,7 @@
 import java.util.List;
 import java.util.Map;
 
+import org.apache.commons.io.FilenameUtils;
 import org.nuxeo.common.utils.StringUtils;
 import org.nuxeo.ecm.core.api.Blob;
 import org.nuxeo.ecm.core.api.blobholder.BlobHolder;
@@ -42,7 +43,8 @@
 import org.nuxeo.runtime.api.Framework;
 
 /**
- * Base class to implement {@link Converter} based on {@link CommandLineExecutorService}.
+ * Base class to implement {@link Converter} based on
+ * {@link CommandLineExecutorService}.
  *
  * @author tiry
  */
@@ -79,18 +81,22 @@ public BlobHolder convert(BlobHolder blobHolder,
 
         String commandName = getCommandName(blobHolder, parameters);
         if (commandName == null) {
-            throw new ConversionException("Unable to determine target CommandLine name");
+            throw new ConversionException(
+                    "Unable to determine target CommandLine name");
         }
 
-        Map<String, Blob> blobParams = getCmdBlobParameters(blobHolder, parameters);
-        Map<String, String> strParams = getCmdStringParameters(blobHolder, parameters);
+        Map<String, Blob> blobParams = getCmdBlobParameters(blobHolder,
+                parameters);
+        Map<String, String> strParams = getCmdStringParameters(blobHolder,
+                parameters);
 
         CmdReturn result = execOnBlob(commandName, blobParams, strParams);
 
         return buildResult(result.output, result.params);
     }
 
-    protected String getCommandName(BlobHolder blobHolder, Map<String, Serializable> parameters) {
+    protected String getCommandName(BlobHolder blobHolder,
+            Map<String, Serializable> parameters) {
         String commandName = initParameters.get(CMD_NAME_PARAMETER);
         if (parameters != null && parameters.containsKey(CMD_NAME_PARAMETER)) {
             commandName = (String) parameters.get(CMD_NAME_PARAMETER);
@@ -102,22 +108,26 @@ protected String getCommandName(BlobHolder blobHolder, Map<String, Serializable>
      * Extracts BlobParameters.
      */
     protected abstract Map<String, Blob> getCmdBlobParameters(
-            BlobHolder blobHolder, Map<String, Serializable> parameters) throws ConversionException;
+            BlobHolder blobHolder, Map<String, Serializable> parameters)
+            throws ConversionException;
 
     /**
      * Extracts String parameters.
      */
     protected abstract Map<String, String> getCmdStringParameters(
-            BlobHolder blobHolder, Map<String, Serializable> parameters) throws ConversionException;
+            BlobHolder blobHolder, Map<String, Serializable> parameters)
+            throws ConversionException;
 
     /**
      * Builds result from commandLine output buffer.
      */
-    protected abstract BlobHolder buildResult(List<String> cmdOutput, CmdParameters cmdParams) throws ConversionException;
+    protected abstract BlobHolder buildResult(List<String> cmdOutput,
+            CmdParameters cmdParams) throws ConversionException;
 
     protected class CmdReturn {
 
         protected final CmdParameters params;
+
         protected final List<String> output;
 
         protected CmdReturn(CmdParameters params, List<String> output) {
@@ -137,7 +147,10 @@ protected CmdReturn execOnBlob(String commandName,
             if (blobParameters != null) {
                 for (String blobParamName : blobParameters.keySet()) {
                     Blob blob = blobParameters.get(blobParamName);
-                    File file = File.createTempFile("cmdLineBasedConverter", blob.getFilename());
+                    File file = File.createTempFile(
+                            "cmdLineBasedConverter",
+                            "."
+                                    + FilenameUtils.getExtension(blob.getFilename()));
                     blob.transferTo(file);
                     params.addNamedParameter(blobParamName, file);
                     filesToDelete.add(file.getAbsolutePath());
@@ -146,22 +159,28 @@ protected CmdReturn execOnBlob(String commandName,
 
             if (parameters != null) {
                 for (String paramName : parameters.keySet()) {
-                    params.addNamedParameter(paramName, parameters.get(paramName));
+                    params.addNamedParameter(paramName,
+                            parameters.get(paramName));
                 }
             }
 
-            ExecResult result = getCommandLineService().execCommand(commandName, params);
+            ExecResult result = getCommandLineService().execCommand(
+                    commandName, params);
 
             if (!result.isSuccessful()) {
-                throw new ConversionException("CommandLine returned code " + result.getReturnCode() + ":\n  " + StringUtils.join(result.getOutput(), "\n  "), result.getError());
+                throw new ConversionException("CommandLine returned code "
+                        + result.getReturnCode() + ":\n  "
+                        + StringUtils.join(result.getOutput(), "\n  "),
+                        result.getError());
             }
 
             return new CmdReturn(params, result.getOutput());
         } catch (CommandNotAvailable e) {
             // XXX bubble installation instructions
             throw new ConversionException("Unable to find targetCommand", e);
         } catch (IOException e) {
-            throw new ConversionException("Error while converting via CommandLineService", e);
+            throw new ConversionException(
+                    "Error while converting via CommandLineService", e);
 
         } finally {
             for (String fileToDelete : filesToDelete) {
@@ -185,12 +204,14 @@ public ConverterCheckResult isConverterAvailable() {
             return new ConverterCheckResult();
         }
 
-        CommandAvailability ca = getCommandLineService().getCommandAvailability(commandName);
+        CommandAvailability ca = getCommandLineService().getCommandAvailability(
+                commandName);
 
         if (ca.isAvailable()) {
             return new ConverterCheckResult();
         } else {
-            return new ConverterCheckResult(ca.getInstallMessage(), ca.getErrorMessage());
+            return new ConverterCheckResult(ca.getInstallMessage(),
+                    ca.getErrorMessage());
         }
     }
 

diff --git a/nuxeo-platform-convert/src/main/resources/OSGI-INF/commandline-pdf2html-contrib.xml b/nuxeo-platform-convert/src/main/resources/OSGI-INF/commandline-pdf2html-contrib.xml
@@ -7,8 +7,8 @@
 
     <command name="pdftohtml" enabled="true">
       <commandLine>pdftohtml</commandLine>
-      <parameterString> -c -enc UTF-8 -noframes #{inFilePath} "#{outDirPath}/index.html"</parameterString>
-      <winParameterString> -c -enc UTF-8 -noframes #{inFilePath} "#{outDirPath}\index.html"</winParameterString>
+      <parameterString> -c -enc UTF-8 -noframes #{inFilePath} #{outDirPath}/index.html</parameterString>
+      <winParameterString> -c -enc UTF-8 -noframes #{inFilePath} #{outDirPath}\index.html</winParameterString>
       <installationDirective>You need to install pdftohtml</installationDirective>
     </command>
 

diff --git a/nuxeo-platform-convert/src/main/resources/OSGI-INF/commandline-pdf2image-contrib.xml b/nuxeo-platform-convert/src/main/resources/OSGI-INF/commandline-pdf2image-contrib.xml
@@ -7,7 +7,7 @@
 
     <command name="pdftoimage" enabled="true">
       <commandLine>convert</commandLine>
-      <parameterString>#{sourceFilePath} "#{targetFilePath}"</parameterString>
+      <parameterString>#{sourceFilePath} #{targetFilePath}</parameterString>
       <installationDirective>You need to install ImageMagick.</installationDirective>
     </command>