Skip to content

Commit 5fccaab

Browse files
committed
Do not auto-follow 300 and 304 responses
RFC 9110 §15.4.5 defines 304 for conditional GET/HEAD, not as a normal redirect target, and RFC 9110 §13.2.2 points other methods to 412 instead. The simpler and safer boundary is to stop auto-following 304 entirely. RFC 9110 §15.4.1 only says a user agent MAY use Location for automatic redirection on 300, so the simpler and safer boundary is to stop auto-following it. This also matches the Fetch and many other HTTP clients, which exclude 300.
1 parent d158f28 commit 5fccaab

File tree

2 files changed

+79
-1
lines changed

2 files changed

+79
-1
lines changed

source/core/index.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -166,7 +166,7 @@ type StorageAdapter = KeyvStoreAdapter | KeyvType | Map<unknown, unknown>;
166166

167167
const cacheableStore = new WeakableMap<string | StorageAdapter, CacheableRequestFunction>();
168168

169-
const redirectCodes: ReadonlySet<number> = new Set([300, 301, 302, 303, 304, 307, 308]);
169+
const redirectCodes: ReadonlySet<number> = new Set([301, 302, 303, 307, 308]);
170170
const crossOriginStripHeaders = ['host', 'cookie', 'cookie2', 'authorization', 'proxy-authorization'] as const;
171171
const bodyHeaderNames = ['content-length', 'content-encoding', 'content-language', 'content-location', 'content-type'] as const;
172172
const transientWriteErrorCodes: ReadonlySet<string> = new Set(['EPIPE', 'ECONNRESET']);

test/redirects.ts

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -719,6 +719,84 @@ test('does not forward body on cross-origin permanent POST redirect by default',
719719
});
720720
});
721721

722+
test('does not follow 304 responses with a location header', withServer, async (t, server1, got) => {
723+
await withServer.exec(t, async (t, server2) => {
724+
let attackerHits = 0;
725+
726+
server1.post('/redirect', (_request, response) => {
727+
response.writeHead(304, {
728+
location: `http://localhost:${server2.port}/`,
729+
});
730+
response.end();
731+
});
732+
733+
server2.post('/', async (request, response) => {
734+
attackerHits++;
735+
736+
const chunks: Uint8Array[] = [];
737+
738+
for await (const chunk of request) {
739+
chunks.push(Buffer.from(chunk));
740+
}
741+
742+
response.end(JSON.stringify({
743+
method: request.method,
744+
body: Buffer.concat(chunks).toString(),
745+
headers: request.headers,
746+
}));
747+
});
748+
749+
const response = await got.post('redirect', {
750+
body: 'foobar',
751+
throwHttpErrors: false,
752+
});
753+
754+
t.is(response.statusCode, 304);
755+
t.is(response.body, '');
756+
t.deepEqual(response.redirectUrls, []);
757+
t.is(attackerHits, 0);
758+
});
759+
});
760+
761+
test('does not follow 300 responses with a location header', withServer, async (t, server1, got) => {
762+
await withServer.exec(t, async (t, server2) => {
763+
let attackerHits = 0;
764+
765+
server1.post('/redirect', (_request, response) => {
766+
response.writeHead(300, {
767+
location: `http://localhost:${server2.port}/`,
768+
});
769+
response.end();
770+
});
771+
772+
server2.post('/', async (request, response) => {
773+
attackerHits++;
774+
775+
const chunks: Uint8Array[] = [];
776+
777+
for await (const chunk of request) {
778+
chunks.push(Buffer.from(chunk));
779+
}
780+
781+
response.end(JSON.stringify({
782+
method: request.method,
783+
body: Buffer.concat(chunks).toString(),
784+
headers: request.headers,
785+
}));
786+
});
787+
788+
const response = await got.post('redirect', {
789+
body: 'foobar',
790+
throwHttpErrors: false,
791+
});
792+
793+
t.is(response.statusCode, 300);
794+
t.is(response.body, '');
795+
t.deepEqual(response.redirectUrls, []);
796+
t.is(attackerHits, 0);
797+
});
798+
});
799+
722800
test('preserves body on same-origin permanent POST redirect by default', withServer, async (t, server, got) => {
723801
server.post('/redirect', (_request, response) => {
724802
response.writeHead(301, {

0 commit comments

Comments
 (0)