From 3337d85404848dbed753145814dc7c85f3c15997 Mon Sep 17 00:00:00 2001 From: Sagar Ganiga <2015sagar.ganiga@ves.ac.in> Date: Tue, 9 Feb 2021 00:21:12 +0530 Subject: [PATCH] fix: Modify doubleEscape to handle path with extra backslashes and launch edge without browser key (#14723) --- packages/launcher/lib/windows/index.ts | 30 +++-- packages/launcher/test/unit/windows_spec.ts | 131 +++++++++++++++++--- 2 files changed, 136 insertions(+), 25 deletions(-) diff --git a/packages/launcher/lib/windows/index.ts b/packages/launcher/lib/windows/index.ts index de419ddf009c..4f07b3065bdf 100644 --- a/packages/launcher/lib/windows/index.ts +++ b/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 { 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 { 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' } } diff --git a/packages/launcher/test/unit/windows_spec.ts b/packages/launcher/test/unit/windows_spec.ts index 1b71235232cb..6176d50fb8d3 100644 --- a/packages/launcher/test/unit/windows_spec.ts +++ b/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,19 +91,22 @@ 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, }), ) }) @@ -111,19 +114,22 @@ describe('windows browser detection', () => { 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) + }) }) })