Fix macOS VNC protocol negotiation (#1012)

scripts/patch-guacamole-lite.cjs

@@ -26,10 +26,31 @@ if (!fs.existsSync(guacdClientPath) || !fs.existsSync(cryptPath)) {
 let guacdClientContent = fs.readFileSync(guacdClientPath, "utf8");
 let cryptContent = fs.readFileSync(cryptPath, "utf8");
 
-// Patch 1: version acceptance list
+// Patch 1: protocol version negotiation.
-const oldVersionCheck = "if (version === '1_0_0' || version === '1_1_0') {";
+// guacamole-lite originally only accepted 1.0.0/1.1.0. Support the protocol
-const newVersionCheck =
+// versions Termix can handle, and conservatively answer future 1.x versions as
-  "if (version === '1_0_0' || version === '1_1_0' || version === '1_3_0' || version === '1_5_0') {";
+// VERSION_1_5_0 so guacd still sees support for `require`/`name` without us
+// claiming support for unknown instructions.
+const oldVersionBlock =
+  "                if (version === '1_0_0' || version === '1_1_0') {\n" +
+  "                    protocolVersion = version;\n" +
+  "                } else {\n" +
+  "                    protocolVersion = '1_1_0';\n" +
+  "                }";
+const oldPatchedVersionBlock =
+  "                if (version === '1_0_0' || version === '1_1_0' || version === '1_3_0' || version === '1_5_0') {\n" +
+  "                    protocolVersion = version;\n" +
+  "                } else {\n" +
+  "                    protocolVersion = '1_1_0';\n" +
+  "                }";
+const newVersionBlock =
+  "                if (version === '1_0_0' || version === '1_1_0' || version === '1_3_0' || version === '1_5_0') {\n" +
+  "                    protocolVersion = version;\n" +
+  "                } else if (/^1_\\d+_0$/.test(version)) {\n" +
+  "                    protocolVersion = '1_5_0';\n" +
+  "                } else {\n" +
+  "                    protocolVersion = '1_1_0';\n" +
+  "                }";
 
 // Patch 2: timezone instruction must be sent for all protocols >= 1.1.0, not just 1.1.0
 const oldTimezone = "if (protocolVersion === '1_1_0') {";
@@ -105,17 +126,23 @@ const newReadyHandler =
 
 let patched = false;
 
-if (!guacdClientContent.includes(newVersionCheck)) {
+if (!guacdClientContent.includes("} else if (/^1_\\d+_0$/.test(version)) {")) {
-  if (!guacdClientContent.includes(oldVersionCheck)) {
+  if (guacdClientContent.includes(oldPatchedVersionBlock)) {
+    guacdClientContent = guacdClientContent.replace(
+      oldPatchedVersionBlock,
+      newVersionBlock,
+    );
+  } else if (guacdClientContent.includes(oldVersionBlock)) {
+    guacdClientContent = guacdClientContent.replace(
+      oldVersionBlock,
+      newVersionBlock,
+    );
+  } else {
     console.log(
       "[patch-guacamole-lite] Version check target not found, skipping",
     );
     process.exit(0);
   }
-  guacdClientContent = guacdClientContent.replace(
-    oldVersionCheck,
-    newVersionCheck,
-  );
   patched = true;
 }
 

scripts/patch-guacamole-lite.test.ts

@@ -1,6 +1,29 @@
 import fs from "node:fs";
+import { createRequire } from "node:module";
 import path from "node:path";
-import { describe, expect, it } from "vitest";
+import { describe, expect, it, vi } from "vitest";
+
+const require = createRequire(import.meta.url);
+const GuacdClient = require("../node_modules/guacamole-lite/lib/GuacdClient.js");
+
+type PatchedGuacdClient = {
+  connectionSettings: Record<string, unknown>;
+  nextArgumentStreamIndex: number;
+  sendInstruction: ReturnType<typeof vi.fn>;
+  sendHandshakeReply: (serverHandshake: string[]) => void;
+  sendRequiredArguments: (params: string[]) => void;
+};
+
+function createPatchedClient(
+  connectionSettings: Record<string, unknown>,
+): PatchedGuacdClient {
+  return Object.assign(Object.create(GuacdClient.prototype), {
+    connectionSettings,
+    logger: { log: vi.fn() },
+    nextArgumentStreamIndex: 0,
+    sendInstruction: vi.fn(),
+  });
+}
 
 describe("patch-guacamole-lite", () => {
   it("handles guacd dynamic argument requests", () => {
@@ -20,4 +43,49 @@ describe("patch-guacamole-lite", () => {
     expect(content).toContain("this.sendInstruction(['blob'");
     expect(content).toContain("this.sendInstruction(['end'");
   });
+
+  it("keeps required-argument support when guacd offers a future 1.x protocol", () => {
+    const client = createPatchedClient({
+      hostname: "192.0.2.10",
+      port: 5900,
+      password: "secret",
+      width: 1280,
+      height: 720,
+      dpi: 96,
+    });
+
+    client.sendHandshakeReply(["VERSION_1_6_0", "hostname", "port"]);
+
+    expect(client.sendInstruction).toHaveBeenCalledWith(["timezone"]);
+    expect(client.sendInstruction).toHaveBeenCalledWith([
+      "name",
+      "guacamole-lite",
+    ]);
+    expect(client.sendInstruction).toHaveBeenCalledWith([
+      "connect",
+      "VERSION_1_5_0",
+      "192.0.2.10",
+      5900,
+    ]);
+  });
+
+  it("answers required credentials through argument value streams", () => {
+    const client = createPatchedClient({
+      username: "",
+      password: "secret",
+    });
+
+    client.sendRequiredArguments(["username", "password"]);
+
+    expect(
+      client.sendInstruction.mock.calls.map(([instruction]) => instruction),
+    ).toEqual([
+      ["argv", 0, "text/plain", "username"],
+      ["blob", 0, ""],
+      ["end", 0],
+      ["argv", 1, "text/plain", "password"],
+      ["blob", 1, Buffer.from("secret", "utf8").toString("base64")],
+      ["end", 1],
+    ]);
+  });
 });