Skip to content

Commit bf66b4e

Browse files
wangmiao0668000666wangmiao0668000666
authored
fix(comfy): bound JSON response reads via readProviderJsonResponse
1 parent f83cdec commit bf66b4e

2 files changed

Lines changed: 176 additions & 5 deletions

File tree

‎extensions/comfy/workflow-runtime.test.ts‎

Lines changed: 171 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,171 @@
1+
// Comfy tests cover workflow-runtime bounded-read delegation.
2+
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
3+
import { readJsonResponseForTest, setComfyFetchGuardForTesting } from "./workflow-runtime.js";
4+
5+
describe("readJsonResponse bounded read (readProviderJsonResponse delegation)", () => {
6+
const fetchMock = vi.fn();
7+
8+
beforeEach(() => {
9+
vi.clearAllMocks();
10+
});
11+
12+
afterEach(() => {
13+
setComfyFetchGuardForTesting(null);
14+
vi.restoreAllMocks();
15+
});
16+
17+
it("cancels oversized JSON body via the 16 MiB provider cap", async () => {
18+
const ONE_MIB = 1024 * 1024;
19+
const TOTAL_CHUNKS = 32;
20+
const chunk = new Uint8Array(ONE_MIB);
21+
22+
let bytesPulled = 0;
23+
let canceled = false;
24+
const oversizedJson = new Response(
25+
new ReadableStream<Uint8Array>({
26+
pull(controller) {
27+
if (bytesPulled >= TOTAL_CHUNKS * ONE_MIB) {
28+
controller.close();
29+
return;
30+
}
31+
bytesPulled += chunk.length;
32+
controller.enqueue(chunk);
33+
},
34+
cancel() {
35+
canceled = true;
36+
},
37+
}),
38+
{ status: 200, headers: { "Content-Type": "application/json" } },
39+
);
40+
41+
const release = vi.fn(async () => {});
42+
fetchMock.mockResolvedValueOnce({ response: oversizedJson, release });
43+
setComfyFetchGuardForTesting(fetchMock);
44+
45+
await expect(
46+
readJsonResponseForTest({
47+
url: "http://127.0.0.1:9999/test",
48+
init: { method: "GET" },
49+
timeoutMs: 10_000,
50+
auditContext: "comfy-test",
51+
errorPrefix: "Comfy test failed",
52+
}),
53+
).rejects.toThrow(/JSON response exceeds 16777216 bytes/);
54+
55+
expect(canceled).toBe(true);
56+
expect(bytesPulled).toBeLessThan(TOTAL_CHUNKS * ONE_MIB);
57+
expect(release).toHaveBeenCalledOnce();
58+
});
59+
60+
it("rejects oversized body with correct error prefix", async () => {
61+
const ONE_MIB = 1024 * 1024;
62+
const chunk = new Uint8Array(ONE_MIB);
63+
64+
let bytesPulled = 0;
65+
let canceled = false;
66+
const oversizedJson = new Response(
67+
new ReadableStream<Uint8Array>({
68+
pull(controller) {
69+
if (bytesPulled >= 32 * ONE_MIB) {
70+
controller.close();
71+
return;
72+
}
73+
bytesPulled += chunk.length;
74+
controller.enqueue(chunk);
75+
},
76+
cancel() {
77+
canceled = true;
78+
},
79+
}),
80+
{ status: 200, headers: { "Content-Type": "application/json" } },
81+
);
82+
83+
const release = vi.fn(async () => {});
84+
fetchMock.mockResolvedValueOnce({ response: oversizedJson, release });
85+
setComfyFetchGuardForTesting(fetchMock);
86+
87+
await expect(
88+
readJsonResponseForTest({
89+
url: "http://127.0.0.1:9999/test",
90+
init: { method: "GET" },
91+
timeoutMs: 10_000,
92+
auditContext: "comfy-test",
93+
errorPrefix: "Comfy test failed",
94+
}),
95+
).rejects.toThrow(/^Comfy test failed: JSON response exceeds 16777216 bytes/);
96+
97+
expect(canceled).toBe(true);
98+
expect(bytesPulled).toBeLessThan(32 * ONE_MIB);
99+
});
100+
101+
it("parses small valid JSON body (negative control)", async () => {
102+
const smallBody = { status: "ok" };
103+
104+
const release = vi.fn(async () => {});
105+
fetchMock.mockResolvedValueOnce({
106+
response: new Response(JSON.stringify(smallBody), {
107+
status: 200,
108+
headers: { "Content-Type": "application/json" },
109+
}),
110+
release,
111+
});
112+
setComfyFetchGuardForTesting(fetchMock);
113+
114+
const result = await readJsonResponseForTest<{ status: string }>({
115+
url: "http://127.0.0.1:9999/test",
116+
init: { method: "GET" },
117+
timeoutMs: 10_000,
118+
auditContext: "comfy-test",
119+
errorPrefix: "Comfy test failed",
120+
});
121+
122+
expect(result.status).toBe("ok");
123+
expect(release).toHaveBeenCalledOnce();
124+
});
125+
126+
it("parses valid JSON with expected comfy response shape (happy path)", async () => {
127+
const comfyResponse = { prompt_id: "abc-123" };
128+
129+
const release = vi.fn(async () => {});
130+
fetchMock.mockResolvedValueOnce({
131+
response: new Response(JSON.stringify(comfyResponse), {
132+
status: 200,
133+
headers: { "Content-Type": "application/json" },
134+
}),
135+
release,
136+
});
137+
setComfyFetchGuardForTesting(fetchMock);
138+
139+
const result = await readJsonResponseForTest<{ prompt_id: string }>({
140+
url: "http://127.0.0.1:9999/test",
141+
init: { method: "GET" },
142+
timeoutMs: 10_000,
143+
auditContext: "comfy-test",
144+
errorPrefix: "Comfy test failed",
145+
});
146+
147+
expect(result.prompt_id).toBe("abc-123");
148+
expect(release).toHaveBeenCalledOnce();
149+
});
150+
151+
it("propagates HTTP error status before reading body", async () => {
152+
const release = vi.fn(async () => {});
153+
fetchMock.mockResolvedValueOnce({
154+
response: new Response(null, { status: 500, statusText: "Internal Server Error" }),
155+
release,
156+
});
157+
setComfyFetchGuardForTesting(fetchMock);
158+
159+
await expect(
160+
readJsonResponseForTest({
161+
url: "http://127.0.0.1:9999/test",
162+
init: { method: "GET" },
163+
timeoutMs: 10_000,
164+
auditContext: "comfy-test",
165+
errorPrefix: "Comfy test failed",
166+
}),
167+
).rejects.toThrow(/Comfy test failed/);
168+
169+
expect(release).toHaveBeenCalledOnce();
170+
});
171+
});

‎extensions/comfy/workflow-runtime.ts‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import { resolveApiKeyForProvider } from "openclaw/plugin-sdk/provider-auth-runt
1212
import {
1313
assertOkOrThrowHttpError,
1414
normalizeBaseUrl,
15+
readProviderJsonResponse,
1516
resolveProviderHttpRequestConfig,
1617
} from "openclaw/plugin-sdk/provider-http";
1718
import { readResponseWithLimit } from "openclaw/plugin-sdk/response-limit-runtime";
@@ -318,16 +319,15 @@ async function readJsonResponse<T>(params: {
318319
});
319320
try {
320321
await assertOkOrThrowHttpError(response, params.errorPrefix);
321-
try {
322-
return (await response.json()) as T;
323-
} catch (cause) {
324-
throw new Error(`${params.errorPrefix}: malformed JSON response`, { cause });
325-
}
322+
return (await readProviderJsonResponse(response, params.errorPrefix)) as T;
326323
} finally {
327324
await release();
328325
}
329326
}
330327

328+
/** @internal Test-only export. */
329+
export const readJsonResponseForTest = readJsonResponse;
330+
331331
function resolveFileExtension(params: { fileName?: string; mimeType?: string }): string {
332332
const extension = extensionForMime(params.mimeType);
333333
if (extension) {

0 commit comments

Comments
 (0)