mirror of
https://github.com/morluto/rea.git
synced 2026-11-15 11:13:21 +00:00
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).
This commit is contained in:
1 parent
f88d8964f9
commit
d8c3be9ba5
4 files changed
+98
-4
No files matched your search
@@ -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,
|
||||
|
||||
@@ -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<JsonValue, GhidraSessionError>
|
||||
>();
|
||||
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" }),
|
||||
]);
|
||||
});
|
||||
@@ -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)),
|
||||
|
||||
@@ -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<typeof responseSchema>;
|
||||
|
||||
|
||||
Reference in new issue
Block a user