fix: url encoding of url-unsafe characters

package.json

@@ -301,6 +301,7 @@
   },
   "devDependencies": {
     "@types/ini": "^1.3.30",
+    "@types/lodash": "^4.14.168",
     "@types/mocha": "^2.2.42",
     "@types/mz": "^0.0.32",
     "@types/node": "^10.12.21",
@@ -312,6 +313,7 @@
   "dependencies": {
     "gitconfiglocal": "^2.1.0",
     "ini": "^1.3.5",
+    "lodash": "^4.17.21",
     "mz": "^2.7.0"
   }
 }

src/extension.ts

@@ -242,10 +242,11 @@ async function githubinator({
     return err("could not find url")
   }
   if (openUrl) {
-    vscode.env.openExternal(vscode.Uri.parse(url))
+    // @ts-ignore This works. Using vscode.Uri.parse double encodes characters which breaks URLs.
+    await vscode.env.openExternal(url)
   }
   if (copyToClipboard) {
-    vscode.env.clipboard.writeText(url)
-    vscode.window.showInformationMessage("URL copied to clipboard.")
+    await vscode.env.clipboard.writeText(url)
+    await vscode.window.showInformationMessage("URL copied to clipboard.")
   }
 }

src/providers.ts

@@ -2,6 +2,7 @@ import * as path from "path"
 import * as url from "url"
 import { IProviderConfig } from "./extension"
 import { cleanHostname } from "./utils"
+import { flatten } from "lodash"
 
 interface IBaseGetUrls {
   readonly selection: [number | null, number | null]
@@ -102,6 +103,12 @@ abstract class BaseProvider {
   abstract getUrls(params: IBaseGetUrls): Promise<IUrlInfo | null>
 }
 
+export function pathJoin(...args: string[]): string {
+  return path.join(
+    ...flatten(args.map(x => x.split("/"))).map(encodeURIComponent),
+  )
+}
+
 export class Github extends BaseProvider {
   DEFAULT_HOSTNAMES = ["github.com"]
   PROVIDER_NAME = "github"
@@ -132,7 +139,7 @@ export class Github extends BaseProvider {
         return null
       }
       const u = new url.URL(
-        path.join(
+        pathJoin(
           repoInfo.org,
           repoInfo.repo,
           mode,
@@ -150,11 +157,11 @@ export class Github extends BaseProvider {
     const blameUrl = createUrl("blame")
     const historyUrl = createUrl("commits", false)
     const compareUrl = new url.URL(
-      path.join(repoInfo.org, repoInfo.repo, "compare", head.value),
+      pathJoin(repoInfo.org, repoInfo.repo, "compare", head.value),
       rootUrl,
     ).toString()
     const prUrl = new url.URL(
-      path.join(repoInfo.org, repoInfo.repo, "pull", "new", head.value),
+      pathJoin(repoInfo.org, repoInfo.repo, "pull", "new", head.value),
       rootUrl,
     ).toString()
     return {
@@ -190,15 +197,15 @@ export class Gitlab extends BaseProvider {
     const lines =
       start != null && end != null ? `L${start + 1}-${end + 1}` : null
     const repoUrl = new url.URL(
-      path.join(repoInfo.org, repoInfo.repo),
+      pathJoin(repoInfo.org, repoInfo.repo),
       rootUrl,
     ).toString()
     const createUrl = (mode: string, hash = true) => {
       if (relativeFilePath == null) {
         return null
       }
       const u = new url.URL(
-        path.join(
+        pathJoin(
           repoInfo.org,
           repoInfo.repo,
           mode,
@@ -216,12 +223,12 @@ export class Gitlab extends BaseProvider {
     const blameUrl = createUrl("blame")
     const historyUrl = createUrl("commits", false)
     const compareUrl = new url.URL(
-      path.join(repoInfo.org, repoInfo.repo, "compare", head.value),
+      pathJoin(repoInfo.org, repoInfo.repo, "compare", head.value),
       rootUrl,
     ).toString()
     // https://gitlab.com/recipeyak/recipeyak/merge_requests/new?merge_request%5Bsource_branch%5D=master
     const prUrl = new url.URL(
-      path.join(repoInfo.org, repoInfo.repo, "merge_requests", "new"),
+      pathJoin(repoInfo.org, repoInfo.repo, "merge_requests", "new"),
       rootUrl,
     )
     prUrl.search = `merge_request%5Bsource_branch%5D=${head.value}`
@@ -258,15 +265,15 @@ export class Bitbucket extends BaseProvider {
     const lines =
       start != null && end != null ? `lines-${start + 1}:${end + 1}` : null
     const repoUrl = new url.URL(
-      path.join(repoInfo.org, repoInfo.repo),
+      pathJoin(repoInfo.org, repoInfo.repo),
       rootUrl,
     ).toString()
     const createUrl = (mode: string, hash = true) => {
       if (relativeFilePath == null) {
         return null
       }
       const u = new url.URL(
-        path.join(
+        pathJoin(
           repoInfo.org,
           repoInfo.repo,
           mode,
@@ -283,7 +290,7 @@ export class Bitbucket extends BaseProvider {
     const blobUrl = createUrl("blob")
     const blameUrl = createUrl("annotate")
     const compareUrl = new url.URL(
-      path.join(
+      pathJoin(
         repoInfo.org,
         repoInfo.repo,
         "branches",
@@ -295,7 +302,7 @@ export class Bitbucket extends BaseProvider {
     const historyUrl = createUrl("history-node", false)
     // "https://bitbucket.org/recipeyak/recipeyak/pull-requests/new?source=db99a912f5c4bffe11d91e163cd78ed96589611b"
     const prUrl = new url.URL(
-      path.join(repoInfo.org, repoInfo.repo, "pull-requests", "new"),
+      pathJoin(repoInfo.org, repoInfo.repo, "pull-requests", "new"),
       rootUrl,
     )
     prUrl.search = `source=${head.value}`
@@ -337,7 +344,7 @@ export class VisualStudio extends BaseProvider {
         ? `&line=${start + 1}&lineEnd=${end + 1}`
         : null
     const repoUrl = new url.URL(
-      path.join(repoInfo.org, "_git", repoInfo.repo),
+      pathJoin(repoInfo.org, "_git", repoInfo.repo),
       rootUrl,
     )
     let filePath = relativeFilePath
@@ -359,12 +366,12 @@ export class VisualStudio extends BaseProvider {
       historyUrl.search = baseSearch + "&_a=history"
     }
     const compareUrl = new url.URL(
-      path.join(repoInfo.org, "_git", repoInfo.repo, "branches"),
+      pathJoin(repoInfo.org, "_git", repoInfo.repo, "branches"),
       rootUrl,
     )
     compareUrl.search = `targetVersion=${version}&_a=commits`
     const prUrl = new url.URL(
-      path.join(repoInfo.org, "_git", repoInfo.repo, "pullrequestcreate"),
+      pathJoin(repoInfo.org, "_git", repoInfo.repo, "pullrequestcreate"),
       rootUrl,
     )
     prUrl.search = `sourceRef=${head.value}`

src/test/providers.test.ts

@@ -5,9 +5,25 @@ import {
   createSha,
   createBranch,
   Github,
+  pathJoin,
 } from "../providers"
 import * as assert from "assert"
 
+suite("utils", async () => {
+  test("pathJoin", () => {
+    assert.strictEqual(
+      pathJoin(
+        "ghost",
+        "ghost.github.io",
+        "blob",
+        "fixit/-#123✅",
+        "C#/C#.Package",
+      ),
+      "ghost/ghost.github.io/blob/fixit--%23123%E2%9C%85/C%23/C%23.Package",
+    )
+  })
+})
+
 suite("Github", async () => {
   test("ssh", async () => {
     for (let url of [

yarn.lock

@@ -12,6 +12,11 @@
   resolved "https://registry.yarnpkg.com/@types/ini/-/ini-1.3.30.tgz#d1485459c9fad84e937414b832a2adb649eab379"
   integrity sha512-2+iF8zPSbpU83UKE+PNd4r/MhwNAdyGpk3H+VMgEH3EhjFZq1kouLgRoZrmIcmoGX97xFvqdS44DkICR5Nz3tQ==
 
+"@types/lodash@^4.14.168":
+  version "4.14.168"
+  resolved "https://registry.yarnpkg.com/@types/lodash/-/lodash-4.14.168.tgz#fe24632e79b7ade3f132891afff86caa5e5ce008"
+  integrity sha512-oVfRvqHV/V6D1yifJbVRU3TMp8OT6o6BG+U9MkwuJ3U8/CsDHvalRpsxBqivn71ztOFZBTfJMvETbqHiaNSj7Q==
+
 "@types/mocha@^2.2.42":
   version "2.2.48"
   resolved "https://registry.yarnpkg.com/@types/mocha/-/mocha-2.2.48.tgz#3523b126a0b049482e1c3c11877460f76622ffab"
@@ -336,6 +341,11 @@ js-yaml@^3.7.0:
     argparse "^1.0.7"
     esprima "^4.0.0"
 
+lodash@^4.17.21:
+  version "4.17.21"
+  resolved "https://registry.yarnpkg.com/lodash/-/lodash-4.17.21.tgz#679591c564c3bffaae8454cf0b3df370c3d6911c"
+  integrity sha512-v2kDEe57lecTulaDIuNTPy3Ry4gLGJ6Z1O3vE1krgXZNrsQ+LFTGHVxVjcXPs17LhbZVGedAJv8XZ1tvj5FvSg==
+
 [email protected], minimatch@^3.0.4:
   version "3.0.4"
   resolved "https://registry.yarnpkg.com/minimatch/-/minimatch-3.0.4.tgz#5166e286457f03306064be5497e8dbb0c3d32083"