mirror of
https://github.com/openclaw/openclaw.git
synced 2026-07-08 22:03:59 +00:00
fix(web-shared): bound response.text() fallback to honor maxBytes (#99884)
* fix(web-shared): bound response.text() fallback to honor maxBytes readResponseText's .text() fallback path ignored the maxBytes option, reading the full body unbounded. When a foreign Response lacks a body stream (no getReader) and no arrayBuffer(), the .text() call was the last resort — but maxBytes was never applied. - Honor maxBytes in the .text() fallback: truncate + set truncated=true - Under-cap responses pass through unchanged - No maxBytes set → original behavior preserved (regression safe) * fix(web-shared): use byte-level truncation in .text() fallback Replace text.length/text.slice (character-level) with TextEncoder/ TextDecoder (byte-level) so the maxBytes cap is respected at byte granularity, consistent with the bounded-stream path at lines 240-298. - bytes.byteLength > maxBytes instead of text.length > maxBytes - TextDecoder().decode(bytes.slice(0, maxBytes)) for byte-safe truncation - bytesRead reports actual bytes, not character count * fix(web-shared): refuse unbounded .text() when maxBytes is set When maxBytes is set the caller expects bounded memory. The .text() fallback path buffers the full body before any post-read check, defeating the cap. Fail-closed by returning empty with truncated=true instead of calling .text() unbounded. Without maxBytes .text() is safe — the caller accepts full allocation. * fix(web-shared): byte-level truncation in .text() fallback with maxBytes The .text() fallback used text.length (characters) and text.slice (char-level) against a byte-level maxBytes cap. Use TextEncoder/ TextDecoder for byte-accurate comparison and truncation. - bytes.byteLength > maxBytes instead of text.length > maxBytes - TextDecoder().decode(bytes.slice(0, maxBytes)) for byte-safe truncation - bytesRead reports actual byte count, not character count * fix(web): fail closed on unbounded response fallbacks * test(web): use streaming response fixtures --------- Co-authored-by: Peter Steinberger <steipete@gmail.com>
This commit is contained in:
@@ -6,7 +6,7 @@ import * as logger from "../../logger.js";
|
||||
import { withFetchPreconnect } from "../../test-utils/fetch-mock.js";
|
||||
import "./web-fetch.test-mocks.js";
|
||||
import { createWebFetchTool } from "./web-fetch.js";
|
||||
import { createBaseWebFetchToolConfig, makeFetchHeaders } from "./web-fetch.test-harness.js";
|
||||
import { createBaseWebFetchToolConfig } from "./web-fetch.test-harness.js";
|
||||
|
||||
const lookupMock = vi.fn();
|
||||
const baseToolConfig = createBaseWebFetchToolConfig({
|
||||
@@ -14,24 +14,20 @@ const baseToolConfig = createBaseWebFetchToolConfig({
|
||||
});
|
||||
|
||||
function markdownResponse(body: string, extraHeaders: Record<string, string> = {}): Response {
|
||||
return {
|
||||
ok: true,
|
||||
return new Response(body, {
|
||||
status: 200,
|
||||
headers: makeFetchHeaders({
|
||||
headers: {
|
||||
"content-type": "text/markdown; charset=utf-8",
|
||||
...extraHeaders,
|
||||
}),
|
||||
text: async () => body,
|
||||
} as Response;
|
||||
},
|
||||
});
|
||||
}
|
||||
|
||||
function htmlResponse(body: string): Response {
|
||||
return {
|
||||
ok: true,
|
||||
return new Response(body, {
|
||||
status: 200,
|
||||
headers: makeFetchHeaders({ "content-type": "text/html; charset=utf-8" }),
|
||||
text: async () => body,
|
||||
} as Response;
|
||||
headers: { "content-type": "text/html; charset=utf-8" },
|
||||
});
|
||||
}
|
||||
|
||||
describe("web_fetch Cloudflare Markdown for Agents", () => {
|
||||
|
||||
@@ -146,4 +146,48 @@ describe("readResponseText", () => {
|
||||
expect(cancel).toHaveBeenCalledTimes(1);
|
||||
expect(releaseLock).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it("does not invoke whole-body fallbacks when maxBytes is set", async () => {
|
||||
const arrayBuffer = vi.fn(async () => new TextEncoder().encode("hello").buffer);
|
||||
const text = vi.fn(async () => "hello");
|
||||
const response = {
|
||||
arrayBuffer,
|
||||
headers: new Headers(),
|
||||
text,
|
||||
} as unknown as Response;
|
||||
|
||||
await expect(readResponseText(response, { maxBytes: 4 })).resolves.toEqual({
|
||||
text: "",
|
||||
truncated: true,
|
||||
bytesRead: 0,
|
||||
});
|
||||
expect(arrayBuffer).not.toHaveBeenCalled();
|
||||
expect(text).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("treats a native bodyless response as empty when maxBytes is set", async () => {
|
||||
await expect(
|
||||
readResponseText(new Response(null, { status: 204 }), { maxBytes: 4 }),
|
||||
).resolves.toEqual({
|
||||
text: "",
|
||||
truncated: false,
|
||||
bytesRead: 0,
|
||||
});
|
||||
});
|
||||
|
||||
it("preserves uncapped text-only fallback byte accounting", async () => {
|
||||
const value = "中文🔥";
|
||||
const text = vi.fn(async () => value);
|
||||
const response = {
|
||||
headers: new Headers(),
|
||||
text,
|
||||
} as unknown as Response;
|
||||
|
||||
await expect(readResponseText(response)).resolves.toEqual({
|
||||
text: value,
|
||||
truncated: false,
|
||||
bytesRead: new TextEncoder().encode(value).byteLength,
|
||||
});
|
||||
expect(text).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -297,6 +297,15 @@ export async function readResponseText(
|
||||
return { text: decodeResponseBytes(res, bytes), truncated, bytesRead };
|
||||
}
|
||||
|
||||
if (maxBytes) {
|
||||
if (res instanceof Response && res.body === null) {
|
||||
return { text: "", truncated: false, bytesRead: 0 };
|
||||
}
|
||||
// Whole-body fallbacks allocate before returning, so they cannot honor a byte cap.
|
||||
// Fail closed instead of making maxBytes a returned-text limit only.
|
||||
return { text: "", truncated: true, bytesRead: 0 };
|
||||
}
|
||||
|
||||
const readBytes = (res as { arrayBuffer?: () => Promise<ArrayBuffer> }).arrayBuffer;
|
||||
if (typeof readBytes === "function") {
|
||||
try {
|
||||
@@ -313,7 +322,8 @@ export async function readResponseText(
|
||||
|
||||
try {
|
||||
const text = await res.text();
|
||||
return { text, truncated: false, bytesRead: text.length };
|
||||
const bytes = new TextEncoder().encode(text);
|
||||
return { text, truncated: false, bytesRead: bytes.byteLength };
|
||||
} catch {
|
||||
return { text: "", truncated: false, bytesRead: 0 };
|
||||
}
|
||||
|
||||
@@ -6,7 +6,6 @@ import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
|
||||
import type { LookupFn } from "../../infra/net/ssrf.js";
|
||||
import { resolveRequestUrl } from "../../plugin-sdk/request-url.js";
|
||||
import { withFetchPreconnect } from "../../test-utils/fetch-mock.js";
|
||||
import { makeFetchHeaders } from "./web-fetch.test-harness.js";
|
||||
const { extractReadableContentMock, resolveWebFetchDefinitionMock } = vi.hoisted(() => ({
|
||||
extractReadableContentMock: vi.fn(),
|
||||
resolveWebFetchDefinitionMock: vi.fn(),
|
||||
@@ -29,37 +28,36 @@ import { WEB_FETCH_SPILL_MAX_CHARS } from "./web-fetch.js";
|
||||
|
||||
const lookupMock = vi.fn();
|
||||
|
||||
type MockResponse = {
|
||||
ok: boolean;
|
||||
status: number;
|
||||
url?: string;
|
||||
headers?: { get: (key: string) => string | null };
|
||||
text?: () => Promise<string>;
|
||||
json?: () => Promise<unknown>;
|
||||
};
|
||||
function responseWithUrl(body: BodyInit, init: ResponseInit, url: string): Response {
|
||||
const response = new Response(body, init);
|
||||
Object.defineProperty(response, "url", { value: url });
|
||||
return response;
|
||||
}
|
||||
|
||||
function htmlResponse(html: string, url = "https://example.com/"): MockResponse {
|
||||
return {
|
||||
ok: true,
|
||||
status: 200,
|
||||
function htmlResponse(html: string, url = "https://example.com/"): Response {
|
||||
return responseWithUrl(
|
||||
html,
|
||||
{
|
||||
status: 200,
|
||||
headers: { "content-type": "text/html; charset=utf-8" },
|
||||
},
|
||||
url,
|
||||
headers: makeFetchHeaders({ "content-type": "text/html; charset=utf-8" }),
|
||||
text: async () => html,
|
||||
};
|
||||
);
|
||||
}
|
||||
|
||||
function textResponse(
|
||||
text: string,
|
||||
url = "https://example.com/",
|
||||
contentType = "text/plain; charset=utf-8",
|
||||
): MockResponse {
|
||||
return {
|
||||
ok: true,
|
||||
status: 200,
|
||||
): Response {
|
||||
return responseWithUrl(
|
||||
text,
|
||||
{
|
||||
status: 200,
|
||||
headers: { "content-type": contentType },
|
||||
},
|
||||
url,
|
||||
headers: makeFetchHeaders({ "content-type": contentType }),
|
||||
text: async () => text,
|
||||
};
|
||||
);
|
||||
}
|
||||
|
||||
function errorHtmlResponse(
|
||||
@@ -67,14 +65,16 @@ function errorHtmlResponse(
|
||||
status = 404,
|
||||
url = "https://example.com/",
|
||||
contentType: string | null = "text/html; charset=utf-8",
|
||||
): MockResponse {
|
||||
return {
|
||||
ok: false,
|
||||
status,
|
||||
): Response {
|
||||
const body = contentType ? html : new TextEncoder().encode(html);
|
||||
return responseWithUrl(
|
||||
body,
|
||||
{
|
||||
status,
|
||||
headers: contentType ? { "content-type": contentType } : undefined,
|
||||
},
|
||||
url,
|
||||
headers: contentType ? makeFetchHeaders({ "content-type": contentType }) : makeFetchHeaders({}),
|
||||
text: async () => html,
|
||||
};
|
||||
);
|
||||
}
|
||||
function installMockFetch(
|
||||
impl: (input: RequestInfo | URL, init?: RequestInit) => Promise<Response>,
|
||||
@@ -111,13 +111,7 @@ function createFetchTool(fetchOverrides: Record<string, unknown> = {}) {
|
||||
|
||||
function installPlainTextFetch(text: string) {
|
||||
installMockFetch((input: RequestInfo | URL) =>
|
||||
Promise.resolve({
|
||||
ok: true,
|
||||
status: 200,
|
||||
headers: makeFetchHeaders({ "content-type": "text/plain" }),
|
||||
text: async () => text,
|
||||
url: resolveRequestUrl(input),
|
||||
} as Response),
|
||||
Promise.resolve(textResponse(text, resolveRequestUrl(input))),
|
||||
);
|
||||
}
|
||||
|
||||
@@ -415,13 +409,7 @@ describe("web_fetch extraction fallbacks", () => {
|
||||
it("enforces maxChars after wrapping", async () => {
|
||||
const longText = "x".repeat(5_000);
|
||||
installMockFetch((input: RequestInfo | URL) =>
|
||||
Promise.resolve({
|
||||
ok: true,
|
||||
status: 200,
|
||||
headers: makeFetchHeaders({ "content-type": "text/plain" }),
|
||||
text: async () => longText,
|
||||
url: resolveRequestUrl(input),
|
||||
} as Response),
|
||||
Promise.resolve(textResponse(longText, resolveRequestUrl(input))),
|
||||
);
|
||||
|
||||
const tool = createFetchTool({
|
||||
@@ -655,13 +643,7 @@ describe("web_fetch extraction fallbacks", () => {
|
||||
it("keeps DNS pinning for web_fetch by default even when HTTP_PROXY is configured", async () => {
|
||||
vi.stubEnv("HTTP_PROXY", "http://127.0.0.1:7890");
|
||||
const mockFetch = installMockFetch((input: RequestInfo | URL) =>
|
||||
Promise.resolve({
|
||||
ok: true,
|
||||
status: 200,
|
||||
headers: makeFetchHeaders({ "content-type": "text/plain" }),
|
||||
text: async () => "proxy body",
|
||||
url: resolveRequestUrl(input),
|
||||
} as Response),
|
||||
Promise.resolve(textResponse("proxy body", resolveRequestUrl(input))),
|
||||
);
|
||||
const tool = createFetchTool({ firecrawl: { enabled: false } });
|
||||
|
||||
@@ -678,13 +660,7 @@ describe("web_fetch extraction fallbacks", () => {
|
||||
it("uses env proxy dispatch for web_fetch when trusted env proxy is explicitly enabled", async () => {
|
||||
vi.stubEnv("HTTP_PROXY", "http://127.0.0.1:7890");
|
||||
const mockFetch = installMockFetch((input: RequestInfo | URL) =>
|
||||
Promise.resolve({
|
||||
ok: true,
|
||||
status: 200,
|
||||
headers: makeFetchHeaders({ "content-type": "text/plain" }),
|
||||
text: async () => "proxy body",
|
||||
url: resolveRequestUrl(input),
|
||||
} as Response),
|
||||
Promise.resolve(textResponse("proxy body", resolveRequestUrl(input))),
|
||||
);
|
||||
const tool = createFetchTool({
|
||||
firecrawl: { enabled: false },
|
||||
@@ -800,13 +776,14 @@ describe("web_fetch extraction fallbacks", () => {
|
||||
});
|
||||
|
||||
it("uses the provider fallback when direct fetch fails", async () => {
|
||||
installMockFetch((_input: RequestInfo | URL) => {
|
||||
return Promise.resolve({
|
||||
ok: false,
|
||||
status: 403,
|
||||
headers: makeFetchHeaders({ "content-type": "text/html" }),
|
||||
text: async () => "blocked",
|
||||
} as Response);
|
||||
installMockFetch((input: RequestInfo | URL) => {
|
||||
return Promise.resolve(
|
||||
responseWithUrl(
|
||||
"blocked",
|
||||
{ status: 403, headers: { "content-type": "text/html" } },
|
||||
resolveRequestUrl(input),
|
||||
),
|
||||
);
|
||||
});
|
||||
|
||||
resolveWebFetchDefinitionMock.mockReturnValue({
|
||||
|
||||
Reference in New Issue
Block a user