Do not spawn a shell to avoid having to deal with fileguards, see ubp…

…orts/ubports-installer#1575

package.json

@@ -6,7 +6,7 @@
   "type": "module",
   "scripts": {
     "test": "npx coverage -x src/cancelable-promise.js -r lcov -r html npm run test-nocover",
-    "test-nocover": "npx mocha -- -R spec './tests/unit-tests/*.js'",
+    "test-nocover": "npx mocha -- -R spec './tests/unit-tests/*.js' './tests/*.js'",
     "lint": "npx eslint tests/**/*.js src/*.js",
     "lint-fix": "npm run lint -- --fix",
     "docs": "npx jsdoc -c jsdoc-config.json",

src/common.js

@@ -35,12 +35,3 @@ export function removeFalsy(obj) {
   }
   return Object.keys(obj).length ? obj : null;
 }
-
-/**
- * Add platform-specific quotes to path string (macos can't handle double quotes)
- * @param {String} file path to guard in quotes
- * @returns {String} guarded path
- */
-export function quotepath(file) {
-  return process.platform == "darwin" ? "'" + file + "'" : '"' + file + '"';
-}

src/fastboot.js

@@ -168,7 +168,7 @@ export class Fastboot extends Tool {
    * @returns {Promise}
    */
   boot(image) {
-    return this.exec("boot", common.quotepath(image))
+    return this.exec("boot", image)
       .then(stdout => {
         return;
       })
@@ -184,7 +184,7 @@ export class Fastboot extends Tool {
    * @returns {Promise}
    */
   update(image, wipe) {
-    return this.exec(wipe ? "-w" : "", "update", common.quotepath(image))
+    return this.exec(wipe ? "-w" : "", "update", image)
       .then(stdout => {
         return;
       })

src/heimdall.js

@@ -17,7 +17,6 @@
  * along with this program.  If not, see <http://www.gnu.org/licenses/>.
  */
 
-import { quotepath } from "./common.js";
 import { Tool } from "./tool.js";
 import { CancelablePromise } from "./cancelable-promise.js";
 
@@ -90,7 +89,7 @@ export class Heimdall extends Tool {
    * @returns {Promise<String>}
    */
   printPit(file) {
-    return this.exec("print-pit", ...(file ? ["--file", quotepath(file)] : []))
+    return this.exec("print-pit", ...(file ? ["--file", file] : []))
       .then(r =>
         r
           .split("\n\nEnding session...")[0]

src/tool.js

@@ -115,7 +115,6 @@ export class Tool extends EventEmitter {
       this.executable,
       [...this.extra, ...args].flat(),
       {
-        shell: true,
         env: {
           ADB_TRACE: "rwx"
         }

tests/test-data/fake fileaccesser.js

@@ -0,0 +1,6 @@
+#!/usr/bin/env node
+
+import fs from "fs";
+
+const [, ...args] = process.argv;
+console.log(fs.statSync(args[1]));

tests/test-data/fake_fileaccesser.js

@@ -0,0 +1,6 @@
+#!/usr/bin/env node
+
+import fs from "fs";
+
+const [, ...args] = process.argv;
+console.log(fs.statSync(args[1]));

tests/test_platform.js

@@ -0,0 +1,59 @@
+"use strict";
+
+/*
+ * Copyright (C) 2017-2019 UBports Foundation <[email protected]>
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+import chai from "chai";
+import sinon from "sinon";
+import sinonChai from "sinon-chai";
+const expect = chai.expect;
+chai.use(sinonChai);
+
+import util from "util";
+import { spawn, execFile } from "child_process";
+const execFilePromise = util.promisify(execFile);
+const spawnPromise = (file, args) =>
+  new Promise((resolve, reject) => {
+    const cp = spawn(file, args);
+    cp.on("exit", code => (code ? reject({ code }) : resolve()));
+  });
+
+describe("platform", function() {
+  ["fake_fileaccesser", "fake fileaccesser"].forEach(e => {
+    describe(e, function() {
+      [
+        { f: execFilePromise, n: "execFile" },
+        { f: spawnPromise, n: "spawn" }
+      ].forEach(({ f, n }) => {
+        it(`${n} should access path without spaces`, function() {
+          return f(`tests/test-data/${e}.js`, ["tests/test-data/test_file"]);
+        });
+        it(`${n} should access path with spaces`, function() {
+          return f(`tests/test-data/${e}.js`, ["tests/test-data/test file"]);
+        });
+        it(`${n} should throw on inaccessible path`, function(done) {
+          f(`tests/test-data/${e}.js`, [
+            "tests/test-data/thisdoesntexist"
+          ]).catch(error => {
+            expect(error).to.haveOwnProperty("code", 1);
+            done();
+          });
+        });
+      });
+    });
+  });
+});

tests/unit-tests/test_common.js

@@ -26,25 +26,6 @@ chai.use(sinonChai);
 import * as common from "../../src/common.js";
 
 describe("Common module", function() {
-  describe("quotepath()", function() {
-    it("should use correct quotes for the platform", function() {
-      expect(common.quotepath("some/path with/ spaces")).to.equal(
-        process.platform == "darwin"
-          ? "'some/path with/ spaces'"
-          : '"some/path with/ spaces"'
-      );
-    });
-    [
-      { platform: "darwin", q: "'" },
-      { platform: "linux", q: '"' },
-      { platform: "win32", q: '"' }
-    ].forEach(p =>
-      it(`should use ${p.q} on ${p.platform}`, function() {
-        sinon.stub(process, "platform").value(p.platform);
-        expect(common.quotepath("a")).to.eql(`${p.q}a${p.q}`);
-      })
-    );
-  });
   describe("removeFalsy()", function() {
     it("should remove falsy values from an object", function() {
       expect(

tests/unit-tests/test_fastboot.js

@@ -28,7 +28,6 @@ chai.use(chaiAsPromised);
 import child_process from "child_process";
 
 import { Fastboot } from "../../src/module.js";
-import * as common from "../../src/common.js";
 import { getAndroidToolPath } from "android-tools-bin";
 import { fastbootErrors } from "../test-data/known_errors.js";
 
@@ -206,7 +205,7 @@ describe("Fastboot module", function() {
         stubExec();
         const fastboot = new Fastboot();
         return fastboot.boot("/path/to/image").then(r => {
-          expectArgs("boot", common.quotepath("/path/to/image"));
+          expectArgs("boot", "/path/to/image");
         });
       });
       it("should reject if booting failed", function() {
@@ -223,22 +222,22 @@ describe("Fastboot module", function() {
       it("should resolve if updating works", function() {
         stubExec();
         const fastboot = new Fastboot();
-        return fastboot.update("/path/to/image").then(r => {
-          expectArgs("", "update", common.quotepath("/path/to/image"));
+        return fastboot.update("/path/to an/image").then(r => {
+          expectArgs("", "update", "/path/to an/image");
         });
       });
       it("should not wipe if not specified", function() {
         stubExec();
         const fastboot = new Fastboot();
         return fastboot.update("/path/to/image").then(r => {
-          expectArgs("", "update", common.quotepath("/path/to/image"));
+          expectArgs("", "update", "/path/to/image");
         });
       });
       it("should wipe if specified", function() {
         stubExec();
         const fastboot = new Fastboot();
         return fastboot.update("/path/to/image", true).then(r => {
-          expectArgs("-w", "update", common.quotepath("/path/to/image"));
+          expectArgs("-w", "update", "/path/to/image");
         });
       });
       it("should reject if updating fails", function() {

tests/unit-tests/test_heimdall.js

@@ -28,7 +28,6 @@ chai.use(chaiAsPromised);
 import child_process from "child_process";
 
 import { Heimdall } from "../../src/module.js";
-import * as common from "../../src/common.js";
 import { getAndroidToolPath } from "android-tools-bin";
 import { heimdallErrors } from "../test-data/known_errors.js";
 
@@ -178,11 +177,7 @@ describe("Heimdall module", function() {
         const heimdall = new Heimdall();
         return heimdall.printPit("/test/test-data/test_file").then(r => {
           expect(r.length).to.eql(3);
-          expectArgs(
-            "print-pit",
-            "--file",
-            common.quotepath("/test/test-data/test_file")
-          );
+          expectArgs("print-pit", "--file", "/test/test-data/test_file");
         });
       });
       it("should reject on error", function() {