Repository: stateful/vscode-runme
Pull request #187 (2023-02-09 23:48:31)

Fix url parsing for git@ urls
This broke with the last release. I am working on e2e tests but feel free to review

christian-bromann: @sourishkrout fyi: I wasn't able to set up a e2e scenario as it seems impossible to overcome the alerts of starting VS Code from Chrome 😞 

Patch:
diff --git a/__mocks__/vscode.ts b/__mocks__/vscode.ts
index 64acfabc..7cbdf4d1 100644
--- a/__mocks__/vscode.ts
+++ b/__mocks__/vscode.ts
@@ -2,6 +2,7 @@ import { readFileSync } from 'node:fs'
 import { join } from 'node:path'
 
 import { vi } from 'vitest'
+import { URI } from 'vscode-uri'
 
 export const notebooks = {
   createNotebookController: vi.fn().mockReturnValue({
@@ -17,7 +18,7 @@ export const notebooks = {
 
 export const Uri = {
   joinPath: vi.fn().mockReturnValue('/foo/bar'),
-  parse: vi.fn(),
+  parse: vi.fn((uri: string) => URI.parse(uri)),
   file: vi.fn().mockReturnValue('')
 }
 
diff --git a/package-lock.json b/package-lock.json
index c5383fe9..889d3302 100644
--- a/package-lock.json
+++ b/package-lock.json
@@ -65,6 +65,7 @@
         "ts-node": "^10.9.1",
         "typescript": "^4.9.4",
         "vitest": "^0.28.1",
+        "vscode-uri": "^3.0.7",
         "wdio-vscode-service": "^5.0.0",
         "webdriverio": "^8.2.3",
         "webpack": "^5.75.0",
diff --git a/package.json b/package.json
index b0998a3b..78a63fb5 100644
--- a/package.json
+++ b/package.json
@@ -379,6 +379,7 @@
     "ts-node": "^10.9.1",
     "typescript": "^4.9.4",
     "vitest": "^0.28.1",
+    "vscode-uri": "^3.0.7",
     "wdio-vscode-service": "^5.0.0",
     "webdriverio": "^8.2.3",
     "webpack": "^5.75.0",
diff --git a/src/extension/handler/uri.ts b/src/extension/handler/uri.ts
index 2b61123b..1204884d 100644
--- a/src/extension/handler/uri.ts
+++ b/src/extension/handler/uri.ts
@@ -6,11 +6,11 @@ import { v4 as uuidv4 } from 'uuid'
 import { TelemetryReporter } from 'vscode-telemetry'
 
 import {
-    getProjectDir, getTargetDirName, waitForProjectCheckout, getSuggestedProjectName, writeBootstrapFile
+    getProjectDir, getTargetDirName, waitForProjectCheckout, getSuggestedProjectName, writeBootstrapFile,
+    parseParams
 } from './utils'
 
 let NEXT_TERM_ID = 0
-const DEFAULT_START_FILE = 'README.md'
 const REGEX_WEB_RESOURCE = /^https?:\/\//
 
 export class RunmeUriHandler implements UriHandler {
@@ -25,9 +25,7 @@ export class RunmeUriHandler implements UriHandler {
         }
 
         if (command === 'setup') {
-            const fileToOpen = Uri.parse(params.get('fileToOpen') || DEFAULT_START_FILE).toString()
-            const repository = params.get('repository')
-
+            const { fileToOpen, repository } = parseParams(params)
             if (!repository && fileToOpen.match(REGEX_WEB_RESOURCE)) {
                 TelemetryReporter.sendTelemetryEvent('extension.uriHandler', { command, type: 'file' })
                 await this._setupFile(fileToOpen)
@@ -35,7 +33,7 @@ export class RunmeUriHandler implements UriHandler {
             }
 
             TelemetryReporter.sendTelemetryEvent('extension.uriHandler', { command, type: 'project' })
-            await this._setupProject(fileToOpen, Uri.parse(repository as string).toString())
+            await this._setupProject(fileToOpen, repository)
             return
         }
 
diff --git a/src/extension/handler/utils.ts b/src/extension/handler/utils.ts
index a524aaaa..51b318c7 100644
--- a/src/extension/handler/utils.ts
+++ b/src/extension/handler/utils.ts
@@ -156,3 +156,25 @@ export function getSuggestedProjectName (repository: string) {
     )
     return
 }
+
+const FILE_PROTOCOL = 'file:///'
+const GIT_SCHEMA = 'git@'
+const DEFAULT_START_FILE = 'README.md'
+export function parseParams (params: URLSearchParams) {
+  try {
+    const fileToOpen = Uri.parse(params.get('fileToOpen') || DEFAULT_START_FILE).toString().replace(FILE_PROTOCOL, '')
+    let repository = params.get('repository')
+
+    if (repository) {
+      repository = (
+        repository.startsWith(GIT_SCHEMA)
+          ? GIT_SCHEMA + Uri.parse(repository.slice(GIT_SCHEMA.length)).toString()
+          : Uri.parse(repository).toString()
+      ).replace(FILE_PROTOCOL, '')
+    }
+
+    return { fileToOpen, repository }
+  } catch (err) {
+    throw new Error(`Failed to parse url parameters: ${(err as Error).message}`)
+  }
+}


Test patch:
diff --git a/tests/e2e/specs/launcher.e2e.ts b/tests/e2e/specs/launcher.e2e.ts
new file mode 100644
index 00000000..ecde7845
--- /dev/null
+++ b/tests/e2e/specs/launcher.e2e.ts
@@ -0,0 +1,7 @@
+describe('Runme VS Code Extension', () => {
+  it('should load successfully', async () => {
+    const workbench = await browser.getWorkbench()
+    const title = await workbench.getTitleBar().getTitle()
+    expect(title).toContain('README.md')
+  })
+})
diff --git a/tests/extension/handler/uri.test.ts b/tests/extension/handler/uri.test.ts
index e3c5e8d7..caf4ceed 100644
--- a/tests/extension/handler/uri.test.ts
+++ b/tests/extension/handler/uri.test.ts
@@ -6,12 +6,13 @@ import { TelemetryReporter } from 'vscode-telemetry'
 
 import { RunmeUriHandler } from '../../../src/extension/handler/uri'
 import {
-    getProjectDir, waitForProjectCheckout, getSuggestedProjectName, writeBootstrapFile
+    getProjectDir, waitForProjectCheckout, getSuggestedProjectName, writeBootstrapFile, parseParams
 } from '../../../src/extension/handler/utils'
 
 vi.mock('vscode')
 vi.mock('vscode-telemetry')
 vi.mock('../../../src/extension/handler/utils', () => ({
+    parseParams: vi.fn().mockReturnValue({}),
     getProjectDir: vi.fn(),
     getTargetDirName: vi.fn(),
     waitForProjectCheckout: vi.fn(),
@@ -57,23 +58,27 @@ describe('RunmeUriHandler', () => {
         })
 
         it('runs _setupProject if command was "setup"', async () => {
-            vi.mocked(Uri.parse).mockReturnValue({ toString: () => 'some url', query: {
-                command: 'setup',
-                fileToOpen: '/sub/file.md',
-                repository: 'git@github.com:/foo/bar'
-            }} as any)
+            vi.mocked(Uri.parse).mockReturnValue({
+              toString: () => 'some url',
+              query: { command: 'setup' }
+            } as any)
+            vi.mocked(parseParams).mockReturnValue({
+              fileToOpen: '/sub/file.md',
+              repository: 'git@github.com:/foo/bar'
+            })
             await handler.handleUri(Uri.parse('vscode://stateful.runme?foo=bar'))
-            expect(handler['_setupProject']).toBeCalledWith('some url', 'some url')
+            expect(handler['_setupProject']).toBeCalledWith('/sub/file.md', 'git@github.com:/foo/bar')
             expect(TelemetryReporter.sendTelemetryEvent)
               .toBeCalledWith('extension.uriHandler', { command: 'setup', type: 'project' })
         })
 
         it('runs _setupFile if command was "setup" but no repository param', async () => {
-            const fileToOpen = 'https://raw.githubusercontent.com/stateful/vscode-runme/main/examples/k8s/README.md'
             vi.mocked(Uri.parse).mockReturnValue({
               toString: () => 'https://some url',
-              query: { command: 'setup', fileToOpen }, fsPath: '/foo/bar' } as any
-            )
+              query: { command: 'setup' },
+              fsPath: '/foo/bar'
+            } as any)
+            vi.mocked(parseParams).mockReturnValue({ fileToOpen: 'https://some url', repository: null })
             await handler.handleUri(Uri.parse('vscode://stateful.runme?foo=bar'))
             expect(handler['_setupFile']).toBeCalledWith('https://some url')
             expect(TelemetryReporter.sendTelemetryEvent)
diff --git a/tests/extension/handler/utils.test.ts b/tests/extension/handler/utils.test.ts
index 01b5d74d..f910346c 100644
--- a/tests/extension/handler/utils.test.ts
+++ b/tests/extension/handler/utils.test.ts
@@ -4,10 +4,9 @@ import { describe, it, expect, vi, beforeEach } from 'vitest'
 import { Uri, workspace, window } from 'vscode'
 
 import {
-    getProjectDir, getTargetDirName, getSuggestedProjectName
+    getProjectDir, getTargetDirName, getSuggestedProjectName, parseParams
 } from '../../../src/extension/handler/utils'
 
-vi.mocked(Uri.parse).mockImplementation((input: any) => input)
 vi.mocked(Uri.joinPath).mockImplementation(
     (base: any, ...input: string[]) => path.join('/some/path', ...input) as any)
 vi.mock('vscode')
@@ -23,14 +22,13 @@ beforeEach(() => {
 
 describe('getProjectDir', () => {
     it('should use a tmp dir if config is not set', async () => {
-        expect(await getProjectDir()).toBe('/tmp/dir')
+        expect(await getProjectDir()).toMatchObject({ path: '/tmp/dir' })
     })
 
     it('should return project dir if existing', async () => {
         config.set('projectDir', 'foobar')
-        vi.mocked(Uri.parse).mockReturnValue('foobar' as any)
         vi.mocked(workspace.fs.stat).mockResolvedValueOnce({} as any)
-        expect(await getProjectDir()).toBe('foobar')
+        expect(await getProjectDir()).toMatchObject({ path: '/foobar' })
     })
 
     it('should return null if user does not want to create new project dir', async () => {
@@ -44,7 +42,7 @@ describe('getProjectDir', () => {
         config.set('projectDir', 'foobar')
         vi.mocked(window.showInformationMessage).mockResolvedValue('Yes' as any)
         vi.mocked(workspace.fs.stat).mockRejectedValueOnce(new Error(''))
-        expect(await getProjectDir()).toBe('foobar')
+        expect(await getProjectDir()).toMatchObject({ path: '/foobar' })
         expect(workspace.fs.createDirectory).toBeCalledTimes(1)
     })
 })
@@ -87,3 +85,13 @@ describe('getSuggestedProjectName', () => {
         expect(window.showErrorMessage).toBeCalledTimes(1)
     })
 })
+
+describe('parseParams', () => {
+  it('should parse params to be used safely', () => {
+    const usp = new URLSearchParams('fileToOpen=foo;bar loo&repository=git@github.com/org/project;foo bar.git')
+    expect(parseParams(usp)).toEqual({
+      fileToOpen: 'foo%3Bbar%20loo',
+      repository: 'git@github.com/org/project%3Bfoo%20bar.git'
+    })
+  })
+})