Merge pull request #145 from vim-denops/fix-bufname

🐛 Fix "URI malformed" error on `bufname` module when a `expr` contains percent encoded characters

Author: lambdalisue

.github/workflows/test.yml

@@ -71,6 +71,7 @@ jobs:
           - vim: "v8.2.3452"
             nvim: "v0.6.0"
     runs-on: ${{ matrix.runner }}
+    timeout-minutes: 5
     steps:
       - run: git config --global core.autocrlf false
         if: runner.os == 'Windows'

denops_std/bufname/README.md

@@ -116,7 +116,7 @@ assertEquals(
     scheme: "denops",
     expr: path.toFileUrl("C:\\Users\John Titor\test.git").pathname,
   }),
-  "denops:///C:/Users/John%20Titor/test.git",
+  "denops:///C:/Users/John%2520Titor/test.git",
 );
 ```
 
@@ -183,10 +183,10 @@ import { assertEquals } from "https://deno.land/std/testing/asserts.ts";
 import * as path from "https://deno.land/std/path/mod.ts";
 import { parse } from "../bufname/mod.ts";
 
-const bufname = parse("denops:///C:/Users/John%20Titor/test.git");
+const bufname = parse("denops:///C:/Users/John%2520Titor/test.git");
 assertEquals(bufname, {
   scheme: "denops",
-  expr: "/C:/Users/John Titor/test.git",
+  expr: "/C:/Users/John%20Titor/test.git",
 });
 // NOTE:
 // Works only on Windows (Use path.win32.fromFileUrl instead on other platforms)

denops_std/bufname/bufname.ts

@@ -85,10 +85,9 @@ export function format(
       `Scheme '${scheme}' contains unusable characters. Only alphabets are allowed.`,
     );
   }
-  const encodedPath = encode(expr).replaceAll(";", "%3B").replaceAll(
-    "#",
-    "%23",
-  );
+  const encodedPath = encode(expr.replaceAll("%", "%25"))
+    .replaceAll(";", "%3B")
+    .replaceAll("#", "%23");
   const suffix1 = params
     ? `;${encode(toURLSearchParams(params).toString())}`
     : "";
@@ -123,13 +122,14 @@ export function parse(bufname: string): Bufname {
     );
   }
   const remain = decode(bufname.substring(`${scheme}://`.length), [
+    "%25", // %
     "%3B", // ;
     "%23", // #
   ]);
   const m2 = remain.match(exprPattern)!;
   const expr = decode(m2[1]);
   const params = m2[2]
-    ? fromURLSearchParams(new URLSearchParams(decode(m2[2])))
+    ? fromURLSearchParams(new URLSearchParams(decode(m2[2], ["%25"])))
     : undefined;
   const fragment = m2[3] ? decode(m2[3]) : undefined;
   return {

denops_std/bufname/bufname_test.ts

@@ -191,14 +191,40 @@ Deno.test("format pass example in README.md", () => {
     "denops:///Users/John Titor/test.git;foo=foo&bar=bar1&bar=bar2#README.md",
   );
 
+  const fileUrl = path.win32.toFileUrl("C:\\Users\\John Titor\\test.git");
+  assertEquals(
+    fileUrl.pathname,
+    "/C:/Users/John%20Titor/test.git",
+  );
   assertEquals(
     format({
       scheme: "denops",
-      expr: path.win32.toFileUrl("C:\\Users\\John Titor\\test.git").pathname,
+      expr: fileUrl.pathname,
     }),
-    "denops:///C:/Users/John%20Titor/test.git",
+    "denops:///C:/Users/John%2520Titor/test.git",
   );
 });
+Deno.test("format encodes '%' in 'expr'", () => {
+  const src = {
+    scheme: "denops",
+    expr: "/hello%world",
+  };
+  const dst = format(src);
+  const exp = "denops:///hello%25world";
+  assertEquals(dst, exp);
+});
+Deno.test("format encodes '%' in 'params'", () => {
+  const src = {
+    scheme: "denops",
+    expr: "/absolute/path/to/worktree",
+    params: {
+      foo: "%foo",
+    },
+  };
+  const dst = format(src);
+  const exp = "denops:///absolute/path/to/worktree;foo=%25foo";
+  assertEquals(dst, exp);
+});
 
 Deno.test("parse throws exception when 'expr' contains unusable characters", () => {
   const src = "denops:///<>|?*";
@@ -375,16 +401,52 @@ Deno.test("parse pass example in README.md", () => {
     },
   );
 
-  const bufname = parse("denops:///C:/Users/John%20Titor/test.git");
+  const bufname = parse("denops:///C:/Users/John%2520Titor/test.git");
   assertEquals(
     bufname,
     {
       scheme: "denops",
-      expr: "/C:/Users/John Titor/test.git",
+      expr: "/C:/Users/John%20Titor/test.git",
     },
   );
   assertEquals(
     path.win32.fromFileUrl(`file://${bufname.expr}`),
     "C:\\Users\\John Titor\\test.git",
   );
 });
+Deno.test("parse decode percent-encoded characters ('%') in 'expr'", () => {
+  const src = "denops:///hello%25world";
+  const dst = parse(src);
+  const exp = {
+    scheme: "denops",
+    expr: "/hello%world",
+  };
+  assertEquals(dst, exp);
+});
+Deno.test("parse decode percent-encoded characters ('%') in 'params'", () => {
+  const src = "denops:///absolute/path/to/worktree;foo=%25foo";
+  const dst = parse(src);
+  const exp = {
+    scheme: "denops",
+    expr: "/absolute/path/to/worktree",
+    params: {
+      foo: "%foo",
+    },
+  };
+  assertEquals(dst, exp);
+});
+Deno.test("parse decode bufname that cause 'URI malformed' on denops-std v3.8.1", () => {
+  const src =
+    "ginlog:///Users/alisue/ghq/github.com/lambdalisue/gin.vim;pretty=%22%25C%28yellow%29%25h%25C%28reset%29+%25C%28magenta%29%5B%25ad%5D%25C%28reset%29%25C%28auto%29%25d%25C%28reset%29+%25s+%25C%28cyan%29%40%25an%25C%28reset%29%22#[]$";
+  const dst = parse(src);
+  const exp = {
+    scheme: "ginlog",
+    expr: "/Users/alisue/ghq/github.com/lambdalisue/gin.vim",
+    params: {
+      pretty:
+        '"%C(yellow)%h%C(reset) %C(magenta)[%ad]%C(reset)%C(auto)%d%C(reset) %s %C(cyan)@%an%C(reset)"',
+    },
+    fragment: "[]$",
+  };
+  assertEquals(dst, exp);
+});

denops_std/bufname/utils.ts

@@ -1,11 +1,22 @@
 // Vim on Windows does not support following characters in bufname
 export const bufnameUnusablePattern = /["<>\|\?\*]/g;
 
-// % encoded character
-const encodedCharacterPattern = /(?:%[0-9a-fA-F]{2})+/g;
-
 /**
- * Encode unusable characters to percent-encoded characters.
+ * Encode only unusable characters to percent-encoded characters.
+ *
+ * This function does not encode the percent character itself to avoid
+ * multiple encoding.
+ * Users must encode the percent character themselves before using this
+ * function like:
+ *
+ * ```
+ * const expr = "%Hello world%";
+ * const encoded = encode(expr.replaceAll("%", "%25"));
+ * ```
+ *
+ * Note that this function is not the inverse of `decode`.
+ * The `decode` function decodes all percent-encoded characters, while this function
+ * encodes a few characters.
  */
 export function encode(expr: string): string {
   expr = expr.replaceAll(
@@ -17,17 +28,17 @@ export function encode(expr: string): string {
 
 /**
  * Decode all percent-encoded characters.
+ *
+ * Note that this function is not the inverse of `encode`.
+ * The `encode` function only encodes a few characters, while this function
+ * decodes all percent-encoded characters.
  */
-export function decode(expr: string, excludes?: string[]): string {
-  excludes = (excludes ?? []).map((v) => v.toUpperCase());
-  expr = expr.replaceAll(
-    encodedCharacterPattern,
-    (m) => {
-      if (excludes?.includes(m.toUpperCase())) {
-        return m;
-      }
-      return decodeURIComponent(m);
-    },
+export function decode(expr: string, excludes: string[] = []): string {
+  excludes = excludes.map((v) => v.toUpperCase());
+  const prefix = excludes.length ? `(?!${excludes.join("|")})` : "";
+  const pattern = new RegExp(
+    `(?:${prefix}%[0-9a-fA-F]{2})+`,
+    "g",
   );
-  return expr;
+  return expr.replaceAll(pattern, decodeURIComponent);
 }

denops_std/bufname/utils_test.ts

@@ -13,14 +13,20 @@ Deno.test("encode does nothing on numeric characters", () => {
   const exp = src;
   assertEquals(dst, exp);
 });
+Deno.test("encode does nothing on the percent character", () => {
+  const src = "%";
+  const dst = encode(src);
+  const exp = "%";
+  assertEquals(dst, exp);
+});
 Deno.test('encode encodes some symbol characters ("<>|?*)', () => {
   const src = " !\"#$%&'()*+,-./:;<=>?@[\\]^`{|}~";
   const dst = encode(src);
   const exp = " !%22#$%&'()%2A+,-./:;%3C=%3E%3F@[\\]^`{%7C}~";
   assertEquals(dst, exp);
 });
 Deno.test("encode does nothing on 日本語", () => {
-  const src = "いろはにへへとちりぬるをわかよたれそつねならむうゐのおくやまけふこえてあさきゆめみしゑひもせす";
+  const src = "いろはにほへとちりぬるをわかよたれそつねならむうゐのおくやまけふこえてあさきゆめみしゑひもせす";
   const dst = encode(src);
   const exp = src;
   assertEquals(dst, exp);
@@ -44,14 +50,26 @@ Deno.test("decode does nothing on numeric characters", () => {
   const exp = src;
   assertEquals(dst, exp);
 });
+Deno.test("decode decodes encoded the percent character", () => {
+  const src = "%25";
+  const dst = decode(src);
+  const exp = "%";
+  assertEquals(dst, exp);
+});
+Deno.test("decode decodes encoded the percent character (only once)", () => {
+  const src = "%2520";
+  const dst = decode(src);
+  const exp = "%20";
+  assertEquals(dst, exp);
+});
 Deno.test('decode decodes encoded characters ("<>|?*)', () => {
   const src = " !%22#$%&'()%2A+,-./:;%3C=%3E%3F@[\\]^`{%7C}~";
   const dst = decode(src);
   const exp = " !\"#$%&'()*+,-./:;<=>?@[\\]^`{|}~";
   assertEquals(dst, exp);
 });
 Deno.test("decode does nothing on 日本語", () => {
-  const src = "いろはにへへとちりぬるをわかよたれそつねならむうゐのおくやまけふこえてあさきゆめみしゑひもせす";
+  const src = "いろはにほへとちりぬるをわかよたれそつねならむうゐのおくやまけふこえてあさきゆめみしゑひもせす";
   const dst = decode(src);
   const exp = src;
   assertEquals(dst, exp);
@@ -64,10 +82,10 @@ Deno.test("decode does nothing on emoji (🥃)", () => {
 });
 Deno.test("decode decodes encoded 日本語", () => {
   const src = encodeURIComponent(
-    "いろはにへへとちりぬるをわかよたれそつねならむうゐのおくやまけふこえてあさきゆめみしゑひもせす",
+    "いろはにほへとちりぬるをわかよたれそつねならむうゐのおくやまけふこえてあさきゆめみしゑひもせす",
   );
   const dst = decode(src);
-  const exp = "いろはにへへとちりぬるをわかよたれそつねならむうゐのおくやまけふこえてあさきゆめみしゑひもせす";
+  const exp = "いろはにほへとちりぬるをわかよたれそつねならむうゐのおくやまけふこえてあさきゆめみしゑひもせす";
   assertEquals(dst, exp);
 });
 Deno.test("decode decodes encoded emoji (🥃)", () => {