Skip to content

Commit 72fe2ab

Browse files
committed
fix: discard provider redirect reply bodies
I try to cancel a present redirect response body before following or rejecting each hop, using the existing disposal helper. The cancellation promise is not awaited and errors are ignored. Cleanup is best-effort, not guaranteed connection release; a custom synchronous cancel can still block. Origin and credential checks, redirect limits and the shared deadline are unchanged. The final non-redirect body stays with the caller. Fixes #1126
1 parent b08503b commit 72fe2ab

2 files changed

Lines changed: 61 additions & 0 deletions

File tree

‎src/providers/request.js‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,10 @@ export async function fetchWithTimeout(fetchImpl, url, init = {}, timeoutMs = RE
1717
// own. A different port on the same host is a different origin too.
1818
const response = await fetchImpl(target.toString(), { ...init, signal, redirect: "manual" });
1919
if (!REDIRECTS.has(response.status)) return response;
20+
// Redirect replies are never read. Try to cancel a present hop body
21+
// before following or failing. The cancellation promise is not awaited
22+
// and errors are ignored; a custom synchronous cancel may still block.
23+
discardResponseBody(response);
2024
if (hops >= MAX_REDIRECTS) throw new Error("Provider request redirected too many times");
2125
const location = response.headers?.get?.("location");
2226
if (!location) throw new Error("Provider request redirected without a location");
Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
import test from "node:test";
2+
import assert from "node:assert/strict";
3+
import { fetchWithTimeout } from "../src/providers/request.js";
4+
5+
function trackedRedirect(cancels, location) {
6+
const body = new ReadableStream({ cancel() { cancels.count += 1; } });
7+
return new Response(body, { status: 302, headers: location === null ? {} : { location } });
8+
}
9+
10+
test("followed redirect hop bodies are discarded", async () => {
11+
const cancels = { count: 0 };
12+
let calls = 0;
13+
const fetchImpl = async () => {
14+
calls += 1;
15+
if (calls < 3) return trackedRedirect(cancels, `/hop${calls}`);
16+
return new Response(JSON.stringify({ ok: true }), { status: 200, headers: { "content-type": "application/json" } });
17+
};
18+
const response = await fetchWithTimeout(fetchImpl, "https://media.example.test/Sessions");
19+
assert.equal(response.status, 200);
20+
assert.deepEqual(await response.json(), { ok: true });
21+
await new Promise((resolve) => setImmediate(resolve));
22+
assert.equal(cancels.count, 2);
23+
});
24+
25+
for (const [mode, expectedCalls, matcher] of [
26+
["loop", 4, /redirected too many times/],
27+
["no-location", 1, /redirected without a location/],
28+
["cross-origin", 1, /outside its origin/],
29+
]) {
30+
test(`failing redirect reply bodies are discarded (${mode})`, async () => {
31+
const cancels = { count: 0 };
32+
let calls = 0;
33+
const fetchImpl = async () => {
34+
calls += 1;
35+
const location = mode === "no-location" ? null : mode === "cross-origin" ? "https://other.invalid/x" : "/loop";
36+
return trackedRedirect(cancels, location);
37+
};
38+
await assert.rejects(fetchWithTimeout(fetchImpl, "https://media.example.test/Sessions"), matcher);
39+
await new Promise((resolve) => setImmediate(resolve));
40+
assert.equal(calls, expectedCalls);
41+
assert.equal(cancels.count, expectedCalls);
42+
});
43+
}
44+
45+
for (const mode of ["throw", "reject", "pending"]) {
46+
test(`redirect cleanup ignores errors and does not await its promise (${mode})`, async () => {
47+
let discarded = false;
48+
const fetchImpl = async () => ({ status: 302, headers: { get: () => "https://other.invalid/x" }, body: { cancel() {
49+
discarded = true;
50+
if (mode === "throw") throw new Error("cleanup failed");
51+
if (mode === "reject") return Promise.reject(new Error("cleanup failed"));
52+
return new Promise(() => {});
53+
} } });
54+
await assert.rejects(fetchWithTimeout(fetchImpl, "https://media.example.test/Sessions"), /outside its origin/);
55+
assert.equal(discarded, true);
56+
});
57+
}

0 commit comments

Comments
 (0)