fix: Modify doubleEscape to handle path with extra backslashes and la…

…unch edge without browser key (#14723)

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)
+ })
  })
 })