commit 273a954c9a4a1a283fb336f140ec86500df6a0cd
parent 802c6f88c0d70f12baa719be3000bd72827db6e9
Author: acreskeyMoz <acreskey@mozilla.com>
Date: Mon, 24 Nov 2025 14:01:02 +0000
Bug 1958245 - Firefox not caching 303 redirects r=necko-reviewers,valentin
Per updated RFCs, we will cache resources from 303 redirects for performance reasons, provided there are explicit cache directives.
https://www.rfc-editor.org/rfc/rfc9110.html#name-303-see-other
Differential Revision: https://phabricator.services.mozilla.com/D272071
Diffstat:
4 files changed, 205 insertions(+), 7 deletions(-)
diff --git a/netwerk/protocol/http/nsHttpChannel.cpp b/netwerk/protocol/http/nsHttpChannel.cpp
@@ -3225,12 +3225,27 @@ nsresult nsHttpChannel::ContinueProcessResponse3(nsresult rv) {
break;
case 301:
case 302:
+ case 303:
case 307:
case 308:
- case 303:
#if 0
case 305: // disabled as a security measure (see bug 187996).
#endif
+ // RFC 9110: 303 responses are cacheable, but only with explicit
+ // freshness information (max-age or Expires). Without these directives,
+ // 303 should not be cached.
+ if (httpStatus == 303) {
+ uint32_t freshnessLifetime = 0;
+ bool hasFreshness =
+ (NS_SUCCEEDED(
+ mResponseHead->ComputeFreshnessLifetime(&freshnessLifetime)) &&
+ freshnessLifetime > 0) ||
+ mResponseHead->HasHeader(nsHttp::Expires);
+ if (mResponseHead->NoStore() || mResponseHead->NoCache() ||
+ !hasFreshness) {
+ CloseCacheEntry(false);
+ }
+ }
// don't store the response body for redirects
MaybeInvalidateCacheEntryForSubsequentGet();
PushRedirectAsyncFunc(&nsHttpChannel::ContinueProcessResponse4);
diff --git a/netwerk/protocol/http/nsHttpResponseHead.cpp b/netwerk/protocol/http/nsHttpResponseHead.cpp
@@ -614,12 +614,13 @@ nsresult nsHttpResponseHead::ComputeFreshnessLifetime(uint32_t* result) {
return NS_OK;
}
- // From RFC 7234 Section 4.2.2, heuristics can only be used on responses
- // without explicit freshness whose status codes are defined as cacheable
- // by default, and those responses without explicit freshness that have been
- // marked as explicitly cacheable.
+ // From RFC 9111 Section 4.2.2
+ // (https://www.rfc-editor.org/rfc/rfc9111.html#name-calculating-heuristic-fresh),
+ // heuristics can only be used on responses without explicit freshness whose
+ // status codes are defined as cacheable by default, and those responses
+ // without explicit freshness that have been marked as explicitly cacheable.
// Note that |MustValidate| handled most of non-cacheable status codes.
- if ((mStatus == 302 || mStatus == 304 || mStatus == 307) &&
+ if ((mStatus == 302 || mStatus == 303 || mStatus == 304 || mStatus == 307) &&
!mCacheControlPublic && !mCacheControlPrivate) {
LOG((
"nsHttpResponseHead::ComputeFreshnessLifetime [this = %p] "
@@ -666,6 +667,7 @@ bool nsHttpResponseHead::MustValidate() {
case 300:
case 301:
case 302:
+ case 303:
case 304:
case 307:
case 308:
@@ -673,7 +675,6 @@ bool nsHttpResponseHead::MustValidate() {
case 410:
break;
// Uncacheable redirects
- case 303:
case 305:
// Other known errors
case 401:
diff --git a/netwerk/test/unit/test_303_redirect_caching.js b/netwerk/test/unit/test_303_redirect_caching.js
@@ -0,0 +1,180 @@
+// ABOUTME: Tests HTTP 303 redirect caching behavior with various cache headers
+// ABOUTME: per RFC 9110 which allows caching 303 with explicit freshness info
+"use strict";
+
+const { HttpServer } = ChromeUtils.importESModule(
+ "resource://testing-common/httpd.sys.mjs"
+);
+
+let httpserver = null;
+
+function make_channel(url) {
+ return NetUtil.newChannel({ uri: url, loadUsingSystemPrincipal: true });
+}
+
+const responseBody = "response body";
+
+function contentHandler(metadata, response) {
+ response.setHeader("Content-Type", "text/plain");
+ response.bodyOutputStream.write(responseBody, responseBody.length);
+}
+
+function setup_test_server() {
+ httpserver = new HttpServer();
+ httpserver.registerPathHandler("/content", contentHandler);
+ httpserver.start(-1);
+}
+
+registerCleanupFunction(async () => {
+ if (httpserver) {
+ await new Promise(resolve => httpserver.stop(resolve));
+ }
+});
+
+function get_url(path) {
+ return `http://localhost:${httpserver.identity.primaryPort}${path}`;
+}
+
+// Test 1: 303 with Cache-Control: max-age should be cached
+add_task(async function test_303_with_max_age() {
+ setup_test_server();
+
+ let requestCount = 0;
+ httpserver.registerPathHandler("/redirect1", (metadata, response) => {
+ requestCount++;
+ response.setStatusLine(metadata.httpVersion, 303, "See Other");
+ response.setHeader("Location", get_url("/content"), false);
+ response.setHeader("Cache-Control", "max-age=3600", false);
+ });
+
+ // First request
+ let chan = make_channel(get_url("/redirect1"));
+ let buffer = await new Promise(resolve => {
+ chan.asyncOpen(new ChannelListener((request, buf) => resolve(buf), null));
+ });
+ Assert.equal(buffer, responseBody);
+ Assert.equal(requestCount, 1, "First request should hit server");
+
+ // Second request - should use cached redirect
+ chan = make_channel(get_url("/redirect1"));
+ buffer = await new Promise(resolve => {
+ chan.asyncOpen(new ChannelListener((request, buf) => resolve(buf), null));
+ });
+ Assert.equal(buffer, responseBody);
+ Assert.equal(requestCount, 1, "Second request should use cached redirect");
+});
+
+// Test 2: 303 with Expires header should be cached
+add_task(async function test_303_with_expires() {
+ let requestCount = 0;
+ httpserver.registerPathHandler("/redirect2", (metadata, response) => {
+ requestCount++;
+ response.setStatusLine(metadata.httpVersion, 303, "See Other");
+ response.setHeader("Location", get_url("/content"), false);
+ // Set Expires to 1 hour in the future
+ let expires = new Date();
+ expires.setHours(expires.getHours() + 1);
+ response.setHeader("Expires", expires.toUTCString(), false);
+ });
+
+ // First request
+ let chan = make_channel(get_url("/redirect2"));
+ let buffer = await new Promise(resolve => {
+ chan.asyncOpen(new ChannelListener((request, buf) => resolve(buf), null));
+ });
+ Assert.equal(buffer, responseBody);
+ Assert.equal(requestCount, 1, "First request should hit server");
+
+ // Second request - should use cached redirect
+ chan = make_channel(get_url("/redirect2"));
+ buffer = await new Promise(resolve => {
+ chan.asyncOpen(new ChannelListener((request, buf) => resolve(buf), null));
+ });
+ Assert.equal(buffer, responseBody);
+ Assert.equal(requestCount, 1, "Second request should use cached redirect");
+});
+
+// Test 3: 303 without cache headers should NOT be cached
+add_task(async function test_303_without_cache_headers() {
+ let requestCount = 0;
+ httpserver.registerPathHandler("/redirect3", (metadata, response) => {
+ requestCount++;
+ response.setStatusLine(metadata.httpVersion, 303, "See Other");
+ response.setHeader("Location", get_url("/content"), false);
+ // No Cache-Control or Expires headers
+ });
+
+ // First request
+ let chan = make_channel(get_url("/redirect3"));
+ let buffer = await new Promise(resolve => {
+ chan.asyncOpen(new ChannelListener((request, buf) => resolve(buf), null));
+ });
+ Assert.equal(buffer, responseBody);
+ Assert.equal(requestCount, 1, "First request should hit server");
+
+ // Second request - should hit server again (not cached)
+ chan = make_channel(get_url("/redirect3"));
+ buffer = await new Promise(resolve => {
+ chan.asyncOpen(new ChannelListener((request, buf) => resolve(buf), null));
+ });
+ Assert.equal(buffer, responseBody);
+ Assert.equal(
+ requestCount,
+ 2,
+ "Second request should hit server (not cached)"
+ );
+});
+
+// Test 4: 303 with Cache-Control: no-store should NOT be cached
+add_task(async function test_303_with_no_store() {
+ let requestCount = 0;
+ httpserver.registerPathHandler("/redirect4", (metadata, response) => {
+ requestCount++;
+ response.setStatusLine(metadata.httpVersion, 303, "See Other");
+ response.setHeader("Location", get_url("/content"), false);
+ response.setHeader("Cache-Control", "no-store", false);
+ });
+
+ // First request
+ let chan = make_channel(get_url("/redirect4"));
+ let buffer = await new Promise(resolve => {
+ chan.asyncOpen(new ChannelListener((request, buf) => resolve(buf), null));
+ });
+ Assert.equal(buffer, responseBody);
+ Assert.equal(requestCount, 1, "First request should hit server");
+
+ // Second request - should hit server again due to no-store
+ chan = make_channel(get_url("/redirect4"));
+ buffer = await new Promise(resolve => {
+ chan.asyncOpen(new ChannelListener((request, buf) => resolve(buf), null));
+ });
+ Assert.equal(buffer, responseBody);
+ Assert.equal(requestCount, 2, "Second request should hit server (no-store)");
+});
+
+// Test 5: 303 with Cache-Control: no-cache should require revalidation
+add_task(async function test_303_with_no_cache() {
+ let requestCount = 0;
+ httpserver.registerPathHandler("/redirect5", (metadata, response) => {
+ requestCount++;
+ response.setStatusLine(metadata.httpVersion, 303, "See Other");
+ response.setHeader("Location", get_url("/content"), false);
+ response.setHeader("Cache-Control", "max-age=3600, no-cache", false);
+ });
+
+ // First request
+ let chan = make_channel(get_url("/redirect5"));
+ let buffer = await new Promise(resolve => {
+ chan.asyncOpen(new ChannelListener((request, buf) => resolve(buf), null));
+ });
+ Assert.equal(buffer, responseBody);
+ Assert.equal(requestCount, 1, "First request should hit server");
+
+ // Second request - should revalidate due to no-cache
+ chan = make_channel(get_url("/redirect5"));
+ buffer = await new Promise(resolve => {
+ chan.asyncOpen(new ChannelListener((request, buf) => resolve(buf), null));
+ });
+ Assert.equal(buffer, responseBody);
+ Assert.equal(requestCount, 2, "Second request should revalidate (no-cache)");
+});
diff --git a/netwerk/test/unit/xpcshell.toml b/netwerk/test/unit/xpcshell.toml
@@ -46,6 +46,8 @@ prefs = [
["test_1073747.js"]
+["test_303_redirect_caching.js"]
+
["test_304_headers.js"]
["test_304_responses.js"]
