Fix reading images from network drives containing non ascii characters

Changelog-entry: (Windows): Fix reading images from network drives containing non ascii characters
Change-type: patch

lib/gui/app/os/windows-network-drives.js

@@ -16,11 +16,68 @@
 
 'use strict'
 
+const Bluebird = require('bluebird')
 const cp = require('child_process')
+const fs = require('fs')
 const _ = require('lodash')
 const os = require('os')
-const path = require('path')
+const Path = require('path')
 const process = require('process')
+const tmp = require('tmp')
+const { promisify } = require('util')
+
+/**
+ * @summary returns { path: String, cleanup: Function }
+ * @function
+ *
+ * @returns {Promise<{ path: String, cleanup: Function }>}
+ *
+ * @example
+ * tmpFileAsync()
+ *   .then({ path, cleanup } => {
+ *     console.log(path)
+ *     cleanup()
+ *   });
+ */
+const tmpFileAsync = () => {
+  return new Promise((resolve, reject) => {
+    const options = {
+
+      // Close the file once it's created
+      discardDescriptor: true,
+
+      // Wmic fails with "Invalid global switch" when the "/output:" switch filename contains a dash ("-")
+      prefix: 'tmp'
+    }
+    tmp.file(options, (error, path, _fd, cleanup) => {
+      if (error) {
+        reject(error)
+      } else {
+        resolve({ path, cleanup })
+      }
+    })
+  })
+}
+
+/**
+ * @summary Disposer for tmpFileAsync, calls cleanup()
+ * @function
+ *
+ * @returns {Disposer<{ path: String, cleanup: Function }>}
+ *
+ * @example
+ * await Bluebird.using(tmpFileDisposer(), ({ path }) => {
+ *   console.log(path);
+ * })
+ */
+const tmpFileDisposer = () => {
+  return Bluebird.resolve(tmpFileAsync())
+    .disposer(({ cleanup }) => {
+      cleanup()
+    })
+}
+
+const readFileAsync = promisify(fs.readFile)
 
 /**
  * @summary Promisified child_process.execFile
@@ -53,6 +110,41 @@ const execFileAsync = async (file, args, options) => {
   })
 }
 
+/**
+ * @summary Returns wmic's output for network drives
+ * @function
+ *
+ * @returns {Promise<String>}
+ *
+ * @example
+ * const output = await getWmicNetworkDrivesOutput()
+ */
+exports.getWmicNetworkDrivesOutput = async () => {
+  // Exported for tests.
+  // Windows's wmic outputs ucs2 encoded data.
+  // When trying to read its stdout from node's execFile, it is always transformed (even if you pass { encoding: 'buffer' })
+  // Information is lost and accented characters become unreadable (with no way to guess what they were).
+  // Because of this, we use the wmic's "/output:" switch that redirects the output to a file.
+  // For some reason wmic doesn't like dashes in filenames, that's why we change the tmp file prefix in tmpFileAsync above.
+  return Bluebird.using(tmpFileDisposer(), async ({ path }) => {
+    await execFileAsync(
+      Path.join(process.env.SystemRoot, 'System32', 'Wbem', 'wmic'),
+      [
+        `/output:${path}`,
+        'path',
+        'Win32_LogicalDisk',
+        'Where',
+        'DriveType="4"',
+        'get',
+        'DeviceID,ProviderName'
+      ],
+      { windowsHide: true, windowsVerbatimArguments: true }
+    )
+    const data = await readFileAsync(path, 'ucs2')
+    return data
+  })
+}
+
 /**
  * @summary returns a Map of drive letter -> network locations on Windows
  * @function
@@ -64,12 +156,8 @@ const execFileAsync = async (file, args, options) => {
  *   .then(console.log);
  */
 const getWindowsNetworkDrives = async () => {
-  const result = await execFileAsync(
-    path.join(process.env.SystemRoot, 'System32', 'Wbem', 'wmic'),
-    [ 'path', 'Win32_LogicalDisk', 'Where', 'DriveType="4"', 'get', 'DeviceID,ProviderName' ],
-    { windowsHide: true, windowsVerbatimArguments: true }
-  )
-  const couples = _.chain(result.stdout)
+  const result = await exports.getWmicNetworkDrivesOutput()
+  const couples = _.chain(result)
     .split('\n')
 
     // Remove header line
@@ -85,7 +173,7 @@ const getWindowsNetworkDrives = async () => {
       const colonPosition = str.indexOf(':')
       // eslint-disable-next-line no-magic-numbers
       if (colonPosition === -1) {
-        throw new Error(`Can't parse wmic output: ${result.stdout}`)
+        throw new Error(`Can't parse wmic output: ${result}`)
       }
       // eslint-disable-next-line no-magic-numbers
       return [ str.slice(0, colonPosition + 1), _.trim(str.slice(colonPosition + 1)) ]

npm-shrinkwrap.json

@@ -5619,6 +5619,17 @@
         "chardet": "^0.4.0",
         "iconv-lite": "^0.4.17",
         "tmp": "^0.0.33"
+      },
+      "dependencies": {
+        "tmp": {
+          "version": "0.0.33",
+          "resolved": "https://registry.npmjs.org/tmp/-/tmp-0.0.33.tgz",
+          "integrity": "sha512-jRCJlojKnZ3addtTOjdIqoRuPEKBvNXcGYqzO6zWZX8KfKEpnGY5jfggJQ3EjKuu8D4bJRr0y+cYJFmYbImXGw==",
+          "dev": true,
+          "requires": {
+            "os-tmpdir": "~1.0.2"
+          }
+        }
       }
     },
     "extglob": {
@@ -12444,12 +12455,11 @@
       }
     },
     "tmp": {
-      "version": "0.0.33",
-      "resolved": "https://registry.npmjs.org/tmp/-/tmp-0.0.33.tgz",
-      "integrity": "sha512-jRCJlojKnZ3addtTOjdIqoRuPEKBvNXcGYqzO6zWZX8KfKEpnGY5jfggJQ3EjKuu8D4bJRr0y+cYJFmYbImXGw==",
-      "dev": true,
+      "version": "0.1.0",
+      "resolved": "https://registry.npmjs.org/tmp/-/tmp-0.1.0.tgz",
+      "integrity": "sha512-J7Z2K08jbGcdA1kkQpJSqLF6T0tdQqpR2pnSUXsIchbPdTI9v3e85cLW0d6WDhwuAleOV71j2xWs8qMPfK7nKw==",
       "requires": {
-        "os-tmpdir": "~1.0.2"
+        "rimraf": "^2.6.3"
       }
     },
     "to-arraybuffer": {

package.json

@@ -71,6 +71,7 @@
     "styled-components": "^3.2.3",
     "styled-system": "^3.1.11",
     "sudo-prompt": "^8.2.3",
+    "tmp": "^0.1.0",
     "uuid": "^3.0.1",
     "xml2js": "^0.4.17"
   },

tests/gui/os/windows-network-drives.spec.js

@@ -18,12 +18,11 @@
 
 const { readFile } = require('fs')
 const os = require('os')
-const cp = require('child_process')
 const m = require('mochainon')
 const { env } = require('process')
 const { promisify } = require('util')
 
-const { replaceWindowsNetworkDriveLetter } = require('../../../lib/gui/app/os/windows-network-drives')
+const wnd = require('../../../lib/gui/app/os/windows-network-drives')
 
 const readFileAsync = promisify(readFile)
 
@@ -32,20 +31,20 @@ describe('Network drives on Windows', () => {
     this.osPlatformStub = m.sinon.stub(os, 'platform')
     this.osPlatformStub.returns('win32')
     const wmicOutput = await readFileAsync('tests/data/wmic-output.txt', { encoding: 'ucs2' })
-    this.execFileStub = m.sinon.stub(cp, 'execFile')
-    this.execFileStub.callsArgWith(3, null, wmicOutput)
+    this.outputStub = m.sinon.stub(wnd, 'getWmicNetworkDrivesOutput')
+    this.outputStub.resolves(wmicOutput)
     this.oldSystemRoot = env.SystemRoot
     env.SystemRoot = 'C:\\Windows'
   })
 
   it('should parse network drive mapping on Windows', async () => {
-    m.chai.expect(await replaceWindowsNetworkDriveLetter('Z:\\some-folder\\some-file'))
-      .to.equal('\\\\192.168.1.1\\Public\\some-folder\\some-file')
+    m.chai.expect(await wnd.replaceWindowsNetworkDriveLetter('Z:\\some-folder\\some-file'))
+      .to.equal('\\\\192.168.1.1\\Publicé\\some-folder\\some-file')
   })
 
   after(() => {
     this.osPlatformStub.restore()
-    this.execFileStub.restore()
+    this.outputStub.restore()
     env.SystemRoot = this.oldSystemRoot
   })
 })