Merge pull request #222 from Microsoft/alexr00/quoteEscape

Better handle escaping quotes

.gitignore

@@ -8,4 +8,5 @@ node_modules/
 builderror.log
 lib/
 npm-debug.log
-package-lock.json
+package-lock.json
+fixtures/space folder/

src/windowsPtyAgent.test.ts

@@ -45,9 +45,6 @@ if (process.platform === 'win32') {
       it('doesn\'t escape multiple backslashes', () => {
         check('asdf \\\\qwer', [], '"asdf \\\\qwer"');
       });
-      it('adds backslashes before quotes', () => {
-        check('"asdf "qwer"', [], '"\\"asdf \\"qwer\\""');
-      });
       it('escapes backslashes before quotes', () => {
         check('asdf \\"qwer', [], '"asdf \\\\\\"qwer"');
       });
@@ -60,6 +57,18 @@ if (process.platform === 'win32') {
       it('joins arguments with spaces', () => {
         check('asdf', ['qwer zxcv', '', '"'], 'asdf "qwer zxcv" "" \\"');
       });
+      it('array argument all in quotes', () => {
+        check('asdf', ['"surounded by quotes"'], 'asdf \\"surounded by quotes\\"');
+      });
+      it('array argument quotes in the middle', () => {
+        check('asdf', ['quotes "in the" middle'], 'asdf "quotes \\"in the\\" middle"');
+      });
+      it('array argument quotes near start', () => {
+        check('asdf', ['"quotes" near start'], 'asdf \\"quotes\\" near start');
+      });
+      it('array argument quotes near end', () => {
+        check('asdf', ['quotes "near end"'], 'asdf quotes \\"near end\\"');
+      });
     });
 
     describe('Args as CommandLine', () => {
@@ -71,5 +80,14 @@ if (process.platform === 'win32') {
         check('file', 'foo \\ba"r \baz', 'file foo \\ba"r \baz');
       });
     });
+
+    describe('Real-world cases', () => {
+      it('quotes within quotes', () => {
+        check('cmd.exe', ['/c', 'powershell -noexit -command \'Set-location \"C:\\user\"\''], 'cmd.exe /c "powershell -noexit -command \'Set-location \\\"C:\\user\\"\'"');
+      });
+      it('space within quotes', () => {
+        check('cmd.exe', ['/k', '"C:\\Users\\alros\\Desktop\\test script.bat"'], 'cmd.exe /k \\"C:\\Users\\alros\\Desktop\\test script.bat\\"');
+      });
+    });
   });
 }

src/windowsPtyAgent.ts

@@ -128,10 +128,14 @@ export function argsToCommandLine(file: string, args: ArgvOrCommandLine): string
       result += ' ';
     }
     const arg = argv[argIndex];
+    // if it is empty or it contains whitespace and is not already quoted
     const quote =
-      arg.indexOf(' ') !== -1 ||
-      arg.indexOf('\t') !== -1 ||
-      arg === '';
+      arg === '' ||
+      (arg.indexOf(' ') !== -1 ||
+      arg.indexOf('\t') !== -1) &&
+      ((arg.length > 1) &&
+      ((arg[0] !== '"') &&
+      (arg[arg.length - 1] !== '"')));
     if (quote) {
       result += '\"';
     }

src/windowsTerminal.test.ts

@@ -2,9 +2,10 @@
  * Copyright (c) 2017, Daniel Imms (MIT License).
  */
 
- import * as fs from 'fs';
+import * as fs from 'fs';
 import * as assert from 'assert';
 import { WindowsTerminal } from './windowsTerminal';
+import * as path from 'path';
 
 if (process.platform === 'win32') {
   describe('WindowsTerminal', () => {
@@ -35,19 +36,27 @@ if (process.platform === 'win32') {
     });
 
     describe('Args as CommandLine', () => {
-      it('should not fail running a shell containing a space in the path', (done) => {
-        const gitBashDefaultPath = 'C:\\Program Files\\Git\\bin\\bash.exe';
-        if (!fs.existsSync(gitBashDefaultPath)) {
+      it('should not fail running a file containing a space in the path', (done) => {
+        const spaceFolder = path.resolve(__dirname, '..', 'fixtures', 'space folder');
+        if (!fs.existsSync(spaceFolder)) {
+          fs.mkdirSync(spaceFolder);
+        }
+
+        const cmdCopiedPath = path.resolve(spaceFolder, 'cmd.exe');
+        const data = fs.readFileSync(`${process.env.windir}\\System32\\cmd.exe`);
+        fs.writeFileSync(cmdCopiedPath, data);
+
+        if (!fs.existsSync(cmdCopiedPath)) {
           // Skip test if git bash isn't installed
           return;
         }
-        const term = new WindowsTerminal(gitBashDefaultPath, '-c "echo helloworld"', {});
+        const term = new WindowsTerminal(cmdCopiedPath, '/c echo "hello world"', {});
         let result = '';
         term.on('data', (data) => {
           result += data;
         });
         term.on('exit', () => {
-          assert.ok(result.indexOf('helloworld') >= 0);
+          assert.ok(result.indexOf('hello world') >= 1);
           done();
         });
       });