From d8c3be9ba52c2ef7cd1e1d06e799404de22acfab Mon Sep 17 00:00:00 2001 From: morluto <76467478+morluto@users.noreply.github.com> Date: Sun, 15 Nov 2026 07:59:58 +0800 Subject: [PATCH] fix(ghidra): deliver the bridge's invalid_request rejection to the caller (#10659) The Java bridge answered an unparseable request with a fixed id 1 and closed the connection. The client ignored that reply because id 1 was already settled, so the caller saw a socket-close process failure instead of invalid_request. The bridge now reserves id 0 for connection-level failures, and the client fails the request in flight with the remote reason (or reports a protocol failure when nothing is in flight). --- bridge/ghidra/ReaGhidraBridge.java | 7 ++- src/ghidra/GhidraResponseRouter.test.ts | 66 +++++++++++++++++++++++++ src/ghidra/GhidraResponseRouter.ts | 22 ++++++++- src/ghidra/protocol.ts | 7 ++- 4 files changed, 98 insertions(+), 4 deletions(-) diff --git a/bridge/ghidra/ReaGhidraBridge.java b/bridge/ghidra/ReaGhidraBridge.java index 9ead56597c..a7d3852744 100644 --- a/bridge/ghidra/ReaGhidraBridge.java +++ b/bridge/ghidra/ReaGhidraBridge.java @@ -416,7 +416,9 @@ public final class ReaGhidraBridge extends HeadlessScript { request = parseRequest(line, descriptor.token); } catch (RuntimeException exception) { - writeFailure(writer, 1, "invalid_request", "Bridge request is invalid"); + // The request id may not have parsed, so the connection-level + // failure uses id 0, which no client request uses. + writeFailure(writer, CONNECTION_FAILURE_ID, "invalid_request", "Bridge request is invalid"); return; } } @@ -5541,6 +5543,9 @@ public final class ReaGhidraBridge extends HeadlessScript { writeResponse(writer, response); } + /** Reserved response id for a failure not tied to one parsed request. */ + private static final int CONNECTION_FAILURE_ID = 0; + private static void writeFailure( BufferedWriter writer, int id, diff --git a/src/ghidra/GhidraResponseRouter.test.ts b/src/ghidra/GhidraResponseRouter.test.ts index 814e893267..bf8bcc5576 100644 --- a/src/ghidra/GhidraResponseRouter.test.ts +++ b/src/ghidra/GhidraResponseRouter.test.ts @@ -49,3 +49,69 @@ it("routes later Ghidra response lines after an unknown response id", async () = ); await expect(response).resolves.toEqual(ok("settled")); }); + +const routerFor = (nextId: number) => { + const pending = new PendingOperations< + number, + Result + >(); + const protocolFailures: GhidraSessionError[] = []; + const router = new GhidraResponseRouter({ + pending, + nextId: () => nextId, + remoteFailure: (failure) => + new GhidraSessionError( + "remote", + failure.message, + {}, + { remoteCode: failure.code }, + ), + protocolFailure: (message, cause) => + protocolFailures.push( + new GhidraSessionError("protocol", message, {}, { cause }), + ), + }); + return { pending, protocolFailures, router }; +}; +const invalidRequestReply = (id: number) => + `{"id":${String(id)},"ok":false,"error":{"code":"invalid_request","message":"Bridge request is invalid"}}`; + +it("fails the request in flight with the bridge's connection-level invalid_request", async () => { + const { pending, protocolFailures, router } = routerFor(5); + const response = pending.wait(4, { + cancelledValue: () => + err(new GhidraSessionError("cancelled", "request cancelled")), + }); + + router.route(invalidRequestReply(0)); + + const settled = await response; + expect( + settled.ok ? undefined : settled.error, + "an unattributable rejection reaches the caller as the remote invalid_request", + ).toMatchObject({ kind: "remote", remoteCode: "invalid_request" }); + expect(protocolFailures).toEqual([]); +}); + +it("reports a connection-level rejection with nothing in flight as a protocol failure", () => { + const { protocolFailures, router } = routerFor(5); + + router.route(invalidRequestReply(0)); + + expect(protocolFailures).toEqual([ + expect.objectContaining({ + kind: "protocol", + message: expect.stringContaining("Bridge request is invalid"), + }), + ]); +}); + +it("never accepts a success on the reserved connection id", () => { + const { protocolFailures, router } = routerFor(5); + + router.route('{"id":0,"ok":true,"result":null}'); + + expect(protocolFailures).toEqual([ + expect.objectContaining({ kind: "protocol" }), + ]); +}); diff --git a/src/ghidra/GhidraResponseRouter.ts b/src/ghidra/GhidraResponseRouter.ts index 7058f01f98..ca6ef0243b 100644 --- a/src/ghidra/GhidraResponseRouter.ts +++ b/src/ghidra/GhidraResponseRouter.ts @@ -2,7 +2,11 @@ import type { JsonValue } from "../domain/jsonValue.js"; import { err, type Result } from "../domain/result.js"; import type { PendingOperations } from "../process/PendingOperations.js"; import type { GhidraSessionError } from "./GhidraSessionError.js"; -import { ghidraResponseResult, parseGhidraResponseLine } from "./protocol.js"; +import { + GHIDRA_CONNECTION_FAILURE_ID, + ghidraResponseResult, + parseGhidraResponseLine, +} from "./protocol.js"; import type { GhidraRemoteError } from "./protocol.js"; /** Dependencies for correlating validated bridge responses. */ @@ -31,12 +35,26 @@ export class GhidraResponseRouter { this.#options.protocolFailure(parsed.error.message, parsed.error); return; } + const result = ghidraResponseResult(parsed.value); + if (parsed.value.id === GHIDRA_CONNECTION_FAILURE_ID && !result.ok) { + // The bridge rejected a request it could not attribute and closes the + // connection, so every request in flight fails with its remote reason. + if (this.#options.pending.size === 0) + this.#options.protocolFailure( + `Ghidra rejected the connection: ${result.error.message}`, + result.error, + ); + else { + const failure = this.#options.remoteFailure(result.error); + this.#options.pending.failAll(() => err(failure)); + } + return; + } if (!this.#options.pending.has(parsed.value.id)) { if (parsed.value.id >= this.#options.nextId()) this.#options.protocolFailure("Ghidra returned an unknown response id"); return; } - const result = ghidraResponseResult(parsed.value); this.#options.pending.settle( parsed.value.id, result.ok ? result : err(this.#options.remoteFailure(result.error)), diff --git a/src/ghidra/protocol.ts b/src/ghidra/protocol.ts index d7465c2228..f3958e0382 100644 --- a/src/ghidra/protocol.ts +++ b/src/ghidra/protocol.ts @@ -11,7 +11,9 @@ const responseSchema = z.discriminatedUnion("ok", [ result: ownedJsonValueSchema, }), z.strictObject({ - id: z.int().positive(), + // Id 0 is reserved for a connection-level failure: the bridge could not + // parse the request, so it cannot name the request id it rejects. + id: z.int().nonnegative(), ok: z.literal(false), error: z .object({ @@ -22,6 +24,9 @@ const responseSchema = z.discriminatedUnion("ok", [ }), ]); +/** Response id the bridge uses for a failure not tied to one request. */ +export const GHIDRA_CONNECTION_FAILURE_ID = 0; + /** Validated response emitted by REA's packaged Ghidra script. */ export type GhidraBridgeResponse = z.infer;