Merge pull request #418 from vim-denops/plugin-name

👍 rejects if the plugin name is invalid

autoload/denops/_internal/plugin.vim

@@ -4,6 +4,7 @@ const s:STATE_LOADED = 'loaded'
 const s:STATE_UNLOADING = 'unloading'
 const s:STATE_FAILED = 'failed'
 
+" NOTE: same as denops/@denops-private/service.ts
 const s:VALID_NAME_PATTERN = '^[-_0-9a-zA-Z]\+$'
 
 let s:plugins = {}

denops/@denops-private/service.ts

@@ -45,6 +45,7 @@ export class Service implements HostService, AsyncDisposable {
     if (!this.#host) {
       throw new Error("No host is bound to the service");
     }
+    assertValidPluginName(name);
     if (this.#plugins.has(name)) {
       if (this.#meta.mode === "debug") {
         console.log(`A denops plugin '${name}' is already loaded. Skip`);
@@ -79,21 +80,24 @@ export class Service implements HostService, AsyncDisposable {
   }
 
   async unload(name: string): Promise<void> {
+    assertValidPluginName(name);
     await this.#unload(name);
   }
 
   async reload(name: string): Promise<void> {
+    assertValidPluginName(name);
     const plugin = await this.#unload(name);
     if (plugin) {
       await this.load(name, plugin.script);
     }
   }
 
-  waitLoaded(name: string): Promise<void> {
+  async waitLoaded(name: string): Promise<void> {
     if (this.#closed) {
-      return Promise.reject(new Error("Service closed"));
+      throw new Error("Service closed");
     }
-    return this.#getWaiter(name).promise;
+    assertValidPluginName(name);
+    await this.#getWaiter(name).promise;
   }
 
   interrupt(reason?: unknown): void {
@@ -111,6 +115,7 @@ export class Service implements HostService, AsyncDisposable {
 
   async dispatch(name: string, fn: string, args: unknown[]): Promise<unknown> {
     try {
+      assertValidPluginName(name);
       return await this.#dispatch(name, fn, args);
     } catch (e) {
       throw toVimError(e);
@@ -128,6 +133,7 @@ export class Service implements HostService, AsyncDisposable {
       throw new Error("No host is bound to the service");
     }
     try {
+      assertValidPluginName(name);
       const r = await this.#dispatch(name, fn, args);
       try {
         await this.#host.call("denops#callback#call", success, r);
@@ -173,6 +179,15 @@ export class Service implements HostService, AsyncDisposable {
   }
 }
 
+// NOTE: same as autoload/denops/_internal/plugin.vim
+const VALID_NAME_PATTERN = /^[-_0-9a-zA-Z]+$/;
+
+function assertValidPluginName(name: string) {
+  if (!VALID_NAME_PATTERN.test(name)) {
+    throw new TypeError(`Invalid plugin name: ${name}`);
+  }
+}
+
 type PluginModule = {
   main: Entrypoint;
 };

denops/@denops-private/service_test.ts

@@ -6,8 +6,10 @@ import {
   assertInstanceOf,
   assertMatch,
   assertNotStrictEquals,
+  assertObjectMatch,
   assertRejects,
   assertStrictEquals,
+  assertStringIncludes,
   assertThrows,
 } from "jsr:@std/assert@^1.0.1";
 import {
@@ -21,6 +23,7 @@ import { toFileUrl } from "jsr:@std/path@^1.0.2/to-file-url";
 import type { Meta } from "jsr:@denops/core@^7.0.0";
 import { promiseState } from "jsr:@lambdalisue/async@^2.1.1";
 import { unimplemented } from "jsr:@lambdalisue/errorutil@^1.1.0";
+import { INVALID_PLUGIN_NAMES } from "/denops-testdata/invalid_plugin_names.ts";
 import { resolveTestDataURL } from "/denops-testdata/resolve.ts";
 import type { Host } from "./denops.ts";
 import { Service } from "./service.ts";
@@ -439,6 +442,26 @@ Deno.test("Service", async (t) => {
         ]);
       });
     });
+
+    for (const [plugin_name, label] of INVALID_PLUGIN_NAMES) {
+      await t.step(`if the plugin name is invalid (${label})`, async (t) => {
+        const service = new Service(meta);
+        service.bind(host);
+        using host_call = stub(host, "call");
+
+        await t.step("rejects", async () => {
+          await assertRejects(
+            () => service.load(plugin_name, scriptValid),
+            TypeError,
+            `Invalid plugin name: ${plugin_name}`,
+          );
+        });
+
+        await t.step("does not calls the host", () => {
+          assertSpyCalls(host_call, 0);
+        });
+      });
+    }
   });
 
   await t.step(".unload()", async (t) => {
@@ -760,6 +783,26 @@ Deno.test("Service", async (t) => {
         ]);
       });
     });
+
+    for (const [plugin_name, label] of INVALID_PLUGIN_NAMES) {
+      await t.step(`if the plugin name is invalid (${label})`, async (t) => {
+        const service = new Service(meta);
+        service.bind(host);
+        using host_call = stub(host, "call");
+
+        await t.step("rejects", async () => {
+          await assertRejects(
+            () => service.unload(plugin_name),
+            TypeError,
+            `Invalid plugin name: ${plugin_name}`,
+          );
+        });
+
+        await t.step("does not calls the host", () => {
+          assertSpyCalls(host_call, 0);
+        });
+      });
+    }
   });
 
   await t.step(".reload()", async (t) => {
@@ -1059,6 +1102,26 @@ Deno.test("Service", async (t) => {
         });
       });
     });
+
+    for (const [plugin_name, label] of INVALID_PLUGIN_NAMES) {
+      await t.step(`if the plugin name is invalid (${label})`, async (t) => {
+        const service = new Service(meta);
+        service.bind(host);
+        using host_call = stub(host, "call");
+
+        await t.step("rejects", async () => {
+          await assertRejects(
+            () => service.reload(plugin_name),
+            TypeError,
+            `Invalid plugin name: ${plugin_name}`,
+          );
+        });
+
+        await t.step("does not calls the host", () => {
+          assertSpyCalls(host_call, 0);
+        });
+      });
+    }
   });
 
   await t.step(".waitLoaded()", async (t) => {
@@ -1145,6 +1208,7 @@ Deno.test("Service", async (t) => {
       using _host_call = stub(host, "call");
 
       const actual = service.waitLoaded("dummy");
+      actual.catch(NOOP);
       await service.close();
 
       assertEquals(await promiseState(actual), "rejected");
@@ -1154,6 +1218,21 @@ Deno.test("Service", async (t) => {
         "Service closed",
       );
     });
+
+    for (const [plugin_name, label] of INVALID_PLUGIN_NAMES) {
+      await t.step(`if the plugin name is invalid (${label})`, async (t) => {
+        const service = new Service(meta);
+        service.bind(host);
+
+        await t.step("rejects", async () => {
+          await assertRejects(
+            () => service.waitLoaded(plugin_name),
+            TypeError,
+            `Invalid plugin name: ${plugin_name}`,
+          );
+        });
+      });
+    }
   });
 
   await t.step(".interrupt()", async (t) => {
@@ -1296,6 +1375,21 @@ Deno.test("Service", async (t) => {
         });
       });
     });
+
+    for (const [plugin_name, label] of INVALID_PLUGIN_NAMES) {
+      await t.step(`if the plugin name is invalid (${label})`, async (t) => {
+        const service = new Service(meta);
+        service.bind(host);
+
+        await t.step("rejects", async () => {
+          const err = await assertRejects(
+            () => service.dispatch(plugin_name, "test", []),
+          );
+          assert(typeof err === "string");
+          assertStringIncludes(err, `Invalid plugin name: ${plugin_name}`);
+        });
+      });
+    }
   });
 
   await t.step(".dispatchAsync()", async (t) => {
@@ -1522,6 +1616,40 @@ Deno.test("Service", async (t) => {
         });
       });
     });
+
+    for (const [plugin_name, label] of INVALID_PLUGIN_NAMES) {
+      await t.step(`if the plugin name is invalid (${label})`, async (t) => {
+        const service = new Service(meta);
+        service.bind(host);
+        using host_call = stub(host, "call");
+
+        await t.step("resolves", async () => {
+          await service.dispatchAsync(
+            plugin_name,
+            "test",
+            ["foo"],
+            "success",
+            "failure",
+          );
+        });
+
+        await t.step("calls 'failure' callback", () => {
+          const err = host_call.calls[0]?.args[2];
+          assert(err && typeof err === "object");
+          assertSpyCall(host_call, 0, {
+            args: [
+              "denops#callback#call",
+              "failure",
+              err,
+            ],
+          });
+          assertObjectMatch(err, {
+            name: "TypeError",
+            message: `Invalid plugin name: ${plugin_name}`,
+          });
+        });
+      });
+    }
   });
 
   await t.step(".close()", async (t) => {

tests/denops/runtime/functions/denops/notify_test.ts

@@ -0,0 +1,61 @@
+import { assertEquals, assertStringIncludes } from "jsr:@std/assert@^1.0.1";
+import { delay } from "jsr:@std/async@^1.0.1/delay";
+import { INVALID_PLUGIN_NAMES } from "/denops-testdata/invalid_plugin_names.ts";
+import { resolveTestDataPath } from "/denops-testdata/resolve.ts";
+import { testHost } from "/denops-testutil/host.ts";
+import { wait } from "/denops-testutil/wait.ts";
+
+const ASYNC_DELAY = 100;
+
+const scriptValid = resolveTestDataPath("dummy_valid_plugin.ts");
+
+testHost({
+  name: "denops#notify()",
+  mode: "all",
+  postlude: [
+    "runtime plugin/denops.vim",
+  ],
+  fn: async ({ host, t, stderr }) => {
+    let outputs: string[] = [];
+    stderr.pipeTo(
+      new WritableStream({ write: (s) => void outputs.push(s) }),
+    ).catch(() => {});
+    await wait(() => host.call("eval", "denops#server#status() ==# 'running'"));
+    await host.call("execute", [
+      "let g:__test_denops_events = []",
+      "autocmd User DenopsPlugin* call add(g:__test_denops_events, expand('<amatch>'))",
+    ], "");
+
+    for (const [plugin_name, label] of INVALID_PLUGIN_NAMES) {
+      await t.step(`if the plugin name is invalid (${label})`, async (t) => {
+        await t.step("does not throw an error", async () => {
+          await host.call("denops#notify", plugin_name, "test", ["foo"]);
+        });
+      });
+    }
+
+    await t.step("if the plugin is loaded", async (t) => {
+      // Load plugin and wait.
+      await host.call("execute", [
+        "let g:__test_denops_events = []",
+        `call denops#plugin#load('dummyLoaded', '${scriptValid}')`,
+      ], "");
+      await wait(async () =>
+        (await host.call("eval", "g:__test_denops_events") as string[])
+          .includes("DenopsPluginPost:dummyLoaded")
+      );
+
+      outputs = [];
+      await host.call("denops#notify", "dummyLoaded", "test", ["foo"]);
+
+      await t.step("returns immediately", () => {
+        assertEquals(outputs, []);
+      });
+
+      await t.step("calls dispatcher method", async () => {
+        await delay(ASYNC_DELAY);
+        assertStringIncludes(outputs.join(""), 'This is test call: ["foo"]');
+      });
+    });
+  },
+});