Windows: fix escaping of external commands

The `escapeshellcmd()` function apparently does not escape spaces within a path on Windows which can result in broken functionality.

While the sniffs and report affected by this are apparently not used that much based on the lack of bug reports, fixing it still seemed like the _right thing to do_.

Noticed while running the unit tests on a fresh install on Windows 10.

At some point over the past years, Node has apparently changed their default install directory on Windows and the order in which they register their paths to the Windows system `PATH`.
This means that `where csslint` may result in a `$cmd` path like `C:\Program Files\nodejs\csslint.cmd`, which would be escaped to `C:^\Program Files^\nodejs^\csslint.cmd` on Windows, which in turn results in a `'C:\Program' is not recognized as an internal or external command, operable program or batch file.` error.

I could have changed the install path for NVM on my machine, but that would just have hidden the underlying issue.

It does appear to be a known issue with the function based on the last two comments in this upstream bug report: https://bugs.php.net/bug.php?id=43261, however as that issue is closed, I don't expect this to be fixed in PHP itself, though it might be worth it to open a new issue upstream about it (as those two comments were left on a closed issue years after the close).

Fixed now by checking an escaped path for unescaped spaces when on Windows and if necessary, escaping them.
The escaping is done in such a way that, even if PHP itself would start escaping these spaces, the `Common::escapeshellcmd()` function will still handle this correctly.

Author: jrfnl

src/Reports/Notifysend.php

@@ -18,6 +18,7 @@
 
 use PHP_CodeSniffer\Config;
 use PHP_CodeSniffer\Files\File;
+use PHP_CodeSniffer\Util\Common;
 
 class Notifysend implements Report
 {
@@ -58,7 +59,7 @@ public function __construct()
     {
         $path = Config::getExecutablePath('notifysend');
         if ($path !== null) {
-            $this->path = escapeshellcmd($path);
+            $this->path = Common::escapeshellcmd($path);
         }
 
         $timeout = Config::getConfigData('notifysend_timeout');

src/Standards/Generic/Sniffs/Debug/CSSLintSniff.php

@@ -12,6 +12,7 @@
 use PHP_CodeSniffer\Config;
 use PHP_CodeSniffer\Files\File;
 use PHP_CodeSniffer\Sniffs\Sniff;
+use PHP_CodeSniffer\Util\Common;
 
 class CSSLintSniff implements Sniff
 {
@@ -54,7 +55,7 @@ public function process(File $phpcsFile, $stackPtr)
 
         $fileName = $phpcsFile->getFilename();
 
-        $cmd = escapeshellcmd($csslintPath).' '.escapeshellarg($fileName).' 2>&1';
+        $cmd = Common::escapeshellcmd($csslintPath).' '.escapeshellarg($fileName).' 2>&1';
         exec($cmd, $output, $retval);
 
         if (is_array($output) === false) {

src/Standards/Generic/Sniffs/Debug/ClosureLinterSniff.php

@@ -12,6 +12,7 @@
 use PHP_CodeSniffer\Config;
 use PHP_CodeSniffer\Files\File;
 use PHP_CodeSniffer\Sniffs\Sniff;
+use PHP_CodeSniffer\Util\Common;
 
 class ClosureLinterSniff implements Sniff
 {
@@ -71,7 +72,7 @@ public function process(File $phpcsFile, $stackPtr)
 
         $fileName = $phpcsFile->getFilename();
 
-        $lintPath = escapeshellcmd($lintPath);
+        $lintPath = Common::escapeshellcmd($lintPath);
         $cmd      = $lintPath.' --nosummary --notime --unix_mode '.escapeshellarg($fileName);
         exec($cmd, $output, $retval);
 

src/Standards/Generic/Sniffs/Debug/ESLintSniff.php

@@ -12,6 +12,7 @@
 use PHP_CodeSniffer\Config;
 use PHP_CodeSniffer\Files\File;
 use PHP_CodeSniffer\Sniffs\Sniff;
+use PHP_CodeSniffer\Util\Common;
 
 class ESLintSniff implements Sniff
 {
@@ -76,7 +77,7 @@ public function process(File $phpcsFile, $stackPtr)
             $eslintOptions[] = '--config '.escapeshellarg($configFile);
         }
 
-        $cmd = escapeshellcmd(escapeshellarg($eslintPath).' '.implode(' ', $eslintOptions).' '.escapeshellarg($filename));
+        $cmd = Common::escapeshellcmd(escapeshellarg($eslintPath).' '.implode(' ', $eslintOptions).' '.escapeshellarg($filename));
 
         // Execute!
         exec($cmd, $stdout, $code);

src/Standards/Generic/Sniffs/Debug/JSHintSniff.php

@@ -13,6 +13,7 @@
 use PHP_CodeSniffer\Config;
 use PHP_CodeSniffer\Files\File;
 use PHP_CodeSniffer\Sniffs\Sniff;
+use PHP_CodeSniffer\Util\Common;
 
 class JSHintSniff implements Sniff
 {
@@ -56,10 +57,10 @@ public function process(File $phpcsFile, $stackPtr)
         }
 
         $fileName   = $phpcsFile->getFilename();
-        $jshintPath = escapeshellcmd($jshintPath);
+        $jshintPath = Common::escapeshellcmd($jshintPath);
 
         if ($rhinoPath !== null) {
-            $rhinoPath = escapeshellcmd($rhinoPath);
+            $rhinoPath = Common::escapeshellcmd($rhinoPath);
             $cmd       = "$rhinoPath \"$jshintPath\" ".escapeshellarg($fileName);
             exec($cmd, $output, $retval);
 

src/Standards/Generic/Sniffs/PHP/SyntaxSniff.php

@@ -13,6 +13,7 @@
 use PHP_CodeSniffer\Config;
 use PHP_CodeSniffer\Files\File;
 use PHP_CodeSniffer\Sniffs\Sniff;
+use PHP_CodeSniffer\Util\Common;
 
 class SyntaxSniff implements Sniff
 {
@@ -53,7 +54,7 @@ public function process(File $phpcsFile, $stackPtr)
         }
 
         $fileName = escapeshellarg($phpcsFile->getFilename());
-        $cmd      = escapeshellcmd($this->phpPath)." -l -d display_errors=1 -d error_prepend_string='' $fileName 2>&1";
+        $cmd      = Common::escapeshellcmd($this->phpPath)." -l -d display_errors=1 -d error_prepend_string='' $fileName 2>&1";
         $output   = shell_exec($cmd);
         $matches  = [];
         if (preg_match('/^.*error:(.*) in .* on line ([0-9]+)/m', trim($output), $matches) === 1) {

src/Standards/Squiz/Sniffs/Debug/JSLintSniff.php

@@ -12,6 +12,7 @@
 use PHP_CodeSniffer\Config;
 use PHP_CodeSniffer\Files\File;
 use PHP_CodeSniffer\Sniffs\Sniff;
+use PHP_CodeSniffer\Util\Common;
 
 class JSLintSniff implements Sniff
 {
@@ -56,8 +57,8 @@ public function process(File $phpcsFile, $stackPtr)
 
         $fileName = $phpcsFile->getFilename();
 
-        $rhinoPath  = escapeshellcmd($rhinoPath);
-        $jslintPath = escapeshellcmd($jslintPath);
+        $rhinoPath  = Common::escapeshellcmd($rhinoPath);
+        $jslintPath = Common::escapeshellcmd($jslintPath);
 
         $cmd = "$rhinoPath \"$jslintPath\" ".escapeshellarg($fileName);
         exec($cmd, $output, $retval);

src/Standards/Squiz/Sniffs/Debug/JavaScriptLintSniff.php

@@ -13,6 +13,7 @@
 use PHP_CodeSniffer\Exceptions\RuntimeException;
 use PHP_CodeSniffer\Files\File;
 use PHP_CodeSniffer\Sniffs\Sniff;
+use PHP_CodeSniffer\Util\Common;
 
 class JavaScriptLintSniff implements Sniff
 {
@@ -56,7 +57,7 @@ public function process(File $phpcsFile, $stackPtr)
 
         $fileName = $phpcsFile->getFilename();
 
-        $cmd = '"'.escapeshellcmd($jslPath).'" -nologo -nofilelisting -nocontext -nosummary -output-format __LINE__:__ERROR__ -process '.escapeshellarg($fileName);
+        $cmd = '"'.Common::escapeshellcmd($jslPath).'" -nologo -nofilelisting -nocontext -nosummary -output-format __LINE__:__ERROR__ -process '.escapeshellarg($fileName);
         $msg = exec($cmd, $output, $retval);
 
         // Variable $exitCode is the last line of $output if no error occurs, on

src/Standards/Zend/Sniffs/Debug/CodeAnalyzerSniff.php

@@ -14,6 +14,7 @@
 use PHP_CodeSniffer\Files\File;
 use PHP_CodeSniffer\Config;
 use PHP_CodeSniffer\Exceptions\RuntimeException;
+use PHP_CodeSniffer\Util\Common;
 
 class CodeAnalyzerSniff implements Sniff
 {
@@ -53,7 +54,7 @@ public function process(File $phpcsFile, $stackPtr)
         // In the command, 2>&1 is important because the code analyzer sends its
         // findings to stderr. $output normally contains only stdout, so using 2>&1
         // will pipe even stderr to stdout.
-        $cmd = escapeshellcmd($analyzerPath).' '.escapeshellarg($fileName).' 2>&1';
+        $cmd = Common::escapeshellcmd($analyzerPath).' '.escapeshellarg($fileName).' 2>&1';
 
         // There is the possibility to pass "--ide" as an option to the analyzer.
         // This would result in an output format which would be easier to parse.

src/Util/Common.php

@@ -239,6 +239,28 @@ public static function isStdinATTY()
     }//end isStdinATTY()
 
 
+    /**
+     * Escape a path to a system command.
+     *
+     * @param string $cmd The path to the system command.
+     *
+     * @return string
+     */
+    public static function escapeshellcmd($cmd)
+    {
+        $cmd = escapeshellcmd($cmd);
+
+        if (strtoupper(substr(PHP_OS, 0, 3)) === 'WIN') {
+            // Spaces are not escaped by escapeshellcmd on Windows, but need to be
+            // for the command to be able to execute.
+            $cmd = preg_replace('`(?<!^) `', '^ ', $cmd);
+        }
+
+        return $cmd;
+
+    }//end escapeshellcmd()
+
+
     /**
      * Prepares token content for output to screen.
      *