fix: Modify doubleEscape to handle path with extra backslashes and launch edge without browser key

packages/launcher/lib/windows/index.ts

@@ -1,6 +1,6 @@
 import fse from 'fs-extra'
 import os from 'os'
-import { join, normalize } from 'path'
+import { join, normalize, win32 } from 'path'
 import { tap, trim, prop } from 'ramda'
 import { get } from 'lodash'
 import { notInstalledErr } from '../errors'
@@ -132,15 +132,17 @@ function getWindowsBrowser (browser: Browser): Promise<FoundBrowser> {
       throw notInstalledErr(browser.name)
     }
 
-    return fse.pathExists(exePath)
+    let path = doubleEscape(exePath)
+
+    return fse.pathExists(path)
     .then((exists) => {
-      log('found %s ?', exePath, exists)
+      log('found %s ?', path, exists)
 
       if (!exists) {
         return tryNextExePath()
       }
 
-      return getVersionString(exePath)
+      return getVersionString(path)
       .then(tap(log))
       .then(getVersion)
       .then((version: string) => {
@@ -163,18 +165,20 @@ function getWindowsBrowser (browser: Browser): Promise<FoundBrowser> {
   return tryNextExePath()
 }
 
-export function getVersionString (path: string) {
-  const doubleEscape = (s: string) => {
-    return s.replace(/\\/g, '\\\\')
-  }
+export function doubleEscape (s: string) {
+  // Converts all types of paths into windows supported double backslash path
+  // Handles any number of \\ in the given path
+  return win32.join(...s.split(win32.sep)).replace(/\\/g, '\\\\')
+}
 
+export function getVersionString (path: string) {
   // on Windows using "--version" seems to always start the full
   // browser, no matter what one does.
 
   const args = [
     'datafile',
     'where',
-    `name="${doubleEscape(path)}"`,
+    `name="${path}"`,
     'get',
     'Version',
     '/value',
@@ -203,15 +207,21 @@ export function getPathData (pathStr: string): PathData {
     const pathParts = path.split(':')
 
     browserKey = pathParts.pop() || ''
-    path = pathParts.join(':')
+    path = doubleEscape(pathParts.join(':'))
 
     return { path, browserKey }
   }
 
+  path = doubleEscape(path)
+
   if (pathStr.indexOf('chrome.exe') > -1) {
     return { path, browserKey: 'chrome' }
   }
 
+  if (pathStr.indexOf('edge.exe') > -1) {
+    return { path, browserKey: 'edge' }
+  }
+
   if (pathStr.indexOf('firefox.exe') > -1) {
     return { path, browserKey: 'firefox' }
   }

packages/launcher/test/unit/windows_spec.ts

@@ -14,7 +14,7 @@ import { detectByPath } from '../../lib/detect'
 import { goalBrowsers } from '../fixtures'
 
 function stubBrowser (path: string, version: string) {
-  path = normalize(path.replace(/\\/g, '\\\\'))
+  path = windowsHelper.doubleEscape(normalize(path))
 
   ;(utils.execa as unknown as SinonStub)
   .withArgs('wmic', ['datafile', 'where', `name="${path}"`, 'get', 'Version', '/value'])
@@ -91,39 +91,45 @@ describe('windows browser detection', () => {
 
   it('works with :browserName format in Windows', () => {
     sinon.stub(os, 'platform').returns('win32')
-    stubBrowser(`${HOMEDIR}/foo/bar/browser.exe`, '100')
+    let path = `${HOMEDIR}/foo/bar/browser.exe`
+    let win10Path = windowsHelper.doubleEscape(path)
 
-    return detectByPath(`${HOMEDIR}/foo/bar/browser.exe:foo-browser`, goalBrowsers as Browser[]).then((browser) => {
+    stubBrowser(path, '100')
+
+    return detectByPath(`${path}:foo-browser`, goalBrowsers as Browser[]).then((browser) => {
       expect(browser).to.deep.equal(
         Object.assign({}, goalBrowsers.find((gb) => {
           return gb.name === 'foo-browser'
         }), {
           displayName: 'Custom Foo Browser',
-          info: `Loaded from ${HOMEDIR}/foo/bar/browser.exe`,
+          info: `Loaded from ${win10Path}`,
           custom: true,
           version: '100',
           majorVersion: 100,
-          path: `${HOMEDIR}/foo/bar/browser.exe`,
+          path: win10Path,
         }),
       )
     })
   })
 
   it('identifies browser if name in path', async () => {
     sinon.stub(os, 'platform').returns('win32')
-    stubBrowser(`${HOMEDIR}/foo/bar/chrome.exe`, '100')
+    let path = `${HOMEDIR}/foo/bar/chrome.exe`
+    let win10Path = windowsHelper.doubleEscape(path)
+
+    stubBrowser(path, '100')
 
-    return detectByPath(`${HOMEDIR}/foo/bar/chrome.exe`).then((browser) => {
+    return detectByPath(path).then((browser) => {
       expect(browser).to.deep.equal(
         Object.assign({}, browsers.find((gb) => {
           return gb.name === 'chrome'
         }), {
           displayName: 'Custom Chrome',
-          info: `Loaded from ${HOMEDIR}/foo/bar/chrome.exe`,
+          info: `Loaded from ${win10Path}`,
           custom: true,
           version: '100',
           majorVersion: 100,
-          path: `${HOMEDIR}/foo/bar/chrome.exe`,
+          path: win10Path,
         }),
       )
     })
@@ -149,24 +155,119 @@ describe('windows browser detection', () => {
 
   context('#getPathData', () => {
     it('returns path and browserKey given path with browser key', () => {
-      const res = windowsHelper.getPathData('C:\\foo\\bar.exe:firefox')
+      const browserPath = 'C:\\foo\\bar.exe'
+      const res = windowsHelper.getPathData(`${browserPath}:firefox`)
+
+      expect(res.path).to.eq(windowsHelper.doubleEscape(browserPath))
+      expect(res.browserKey).to.eq('firefox')
+    })
+
+    it('returns path and browserKey given path with a lot of slashes plus browser key', () => {
+      const browserPath = 'C:\\\\\\\\foo\\\\\\bar.exe'
+      const res = windowsHelper.getPathData(`${browserPath}:firefox`)
 
-      expect(res.path).to.eq('C:\\foo\\bar.exe')
+      expect(res.path).to.eq(windowsHelper.doubleEscape(browserPath))
+      expect(res.browserKey).to.eq('firefox')
+    })
+
+    it('returns path and browserKey given nix path with browser key', () => {
+      const browserPath = 'C:/foo/bar.exe'
+      const res = windowsHelper.getPathData(`${browserPath}:firefox`)
+
+      expect(res.path).to.eq(windowsHelper.doubleEscape(browserPath))
       expect(res.browserKey).to.eq('firefox')
     })
 
     it('returns path and chrome given just path', () => {
-      const res = windowsHelper.getPathData('C:\\foo\\bar\\chrome.exe')
+      const browserPath = 'C:\\foo\\bar\\chrome.exe'
+      const res = windowsHelper.getPathData(browserPath)
+
+      expect(res.path).to.eq(windowsHelper.doubleEscape(browserPath))
+      expect(res.browserKey).to.eq('chrome')
+    })
+
+    it('returns path and chrome given just nix path', () => {
+      const browserPath = 'C:/foo/bar/chrome.exe'
+      const res = windowsHelper.getPathData(browserPath)
 
-      expect(res.path).to.eq('C:\\foo\\bar\\chrome.exe')
+      expect(res.path).to.eq(windowsHelper.doubleEscape(browserPath))
       expect(res.browserKey).to.eq('chrome')
     })
 
+    it('returns path and edge given just path for edge', () => {
+      const browserPath = 'C:\\foo\\bar\\edge.exe'
+      const res = windowsHelper.getPathData(browserPath)
+
+      expect(res.path).to.eq(windowsHelper.doubleEscape(browserPath))
+      expect(res.browserKey).to.eq('edge')
+    })
+
+    it('returns path and edge given just path for msedge', () => {
+      const browserPath = 'C:\\foo\\bar\\msedge.exe'
+      const res = windowsHelper.getPathData(browserPath)
+
+      expect(res.path).to.eq(windowsHelper.doubleEscape(browserPath))
+      expect(res.browserKey).to.eq('edge')
+    })
+
+    it('returns path and edge given just nix path', () => {
+      const browserPath = 'C:/foo/bar/edge.exe'
+      const res = windowsHelper.getPathData(browserPath)
+
+      expect(res.path).to.eq(windowsHelper.doubleEscape(browserPath))
+      expect(res.browserKey).to.eq('edge')
+    })
+
+    it('returns path and edge given just nix path for msedge', () => {
+      const browserPath = 'C:/foo/bar/msedge.exe'
+      const res = windowsHelper.getPathData(browserPath)
+
+      expect(res.path).to.eq(windowsHelper.doubleEscape(browserPath))
+      expect(res.browserKey).to.eq('edge')
+    })
+
     it('returns path and firefox given just path', () => {
-      const res = windowsHelper.getPathData('C:\\foo\\bar\\firefox.exe')
+      const browserPath = 'C:\\foo\\bar\\firefox.exe'
+      const res = windowsHelper.getPathData(browserPath)
 
-      expect(res.path).to.eq('C:\\foo\\bar\\firefox.exe')
+      expect(res.path).to.eq(windowsHelper.doubleEscape(browserPath))
       expect(res.browserKey).to.eq('firefox')
     })
+
+    it('returns path and firefox given just nix path', () => {
+      const browserPath = 'C:/foo/bar/firefox.exe'
+      const res = windowsHelper.getPathData(browserPath)
+
+      expect(res.path).to.eq(windowsHelper.doubleEscape(browserPath))
+      expect(res.browserKey).to.eq('firefox')
+    })
+  })
+
+  context('#doubleEscape', () => {
+    let winPath = 'C:\\\\foo\\\\bar.exe'
+
+    it('converts nix path into double escaped win path', async () => {
+      let nixPath = 'C:/foo/bar.exe'
+
+      expect(windowsHelper.doubleEscape(nixPath)).to.eq(winPath)
+    })
+
+    it('converts win path with different backslash combination into double escaped win path', async () => {
+      let badWinPath = 'C:\\\\\\\\\\foo\\bar.exe'
+
+      expect(windowsHelper.doubleEscape(badWinPath)).to.eq(winPath)
+    })
+
+    it('converts single escaped win path into double escaped win path', async () => {
+      let badWinPath = 'C:\\foo\\bar.exe'
+
+      expect(windowsHelper.doubleEscape(badWinPath)).to.eq(winPath)
+    })
+
+    it('does not affect an already double escaped win path', async () => {
+      let badWinPath = 'C:\\\\foo\\\\bar.exe'
+
+      expect(windowsHelper.doubleEscape(badWinPath)).to.eq(badWinPath)
+    })
   })
 })