Skip to content

Commit

Permalink
LibWeb/Fetch: Set HTTP status code on cached responses
Browse files Browse the repository at this point in the history
This change causes HTTP status codes to be set on cached HTTP responses.

Otherwise, without this change, no status codes at all are set on cached
HTTP responses — which causes all cached responses to default to being
loaded/served with a 200 status code. And as a result of that, if the
cached response is from a 30x redirect, then without this change, when
that cached 30x response is loaded, we don’t follow the redirect —
because we see a 200 status, rather than the expected/original 30x.

Fixes LadybirdBrowser/ladybird#863

Note that this change also reverts the temporary workaround added in
LadybirdBrowser/ladybird@f735c464d3f
(LadybirdBrowser/ladybird#899).

(cherry picked from commit 23da1752b50568f2c49b1c63c2777ddffddaf6f5)
  • Loading branch information
sideshowbarker authored and nico committed Nov 8, 2024
1 parent 6264698 commit 7959c42
Showing 1 changed file with 3 additions and 4 deletions.
7 changes: 3 additions & 4 deletions Userland/Libraries/LibWeb/Fetch/Fetching/Fetching.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1307,6 +1307,7 @@ class CachedResponse : public RefCounted<CachedResponse> {
ByteBuffer body;
Infrastructure::Response::BodyInfo body_info;
ByteBuffer method;
Infrastructure::Status status;
URL::URL url;
UnixDateTime current_age;
};
Expand Down Expand Up @@ -1347,6 +1348,7 @@ class CachePartition : public RefCounted<CachePartition> {
auto response = Infrastructure::Response::create(realm.vm());
response->set_body(body);
response->set_body_info(cached_response.body_info);
response->set_status(cached_response.status);
for (auto& [name, value] : cached_response.headers.headers()) {
response->header_list()->append(Infrastructure::Header::from_string_pair(name, value));
}
Expand All @@ -1363,6 +1365,7 @@ class CachePartition : public RefCounted<CachePartition> {
cached_response->body = response.body()->source().get<ByteBuffer>();
cached_response->body_info = response.body_info();
cached_response->method = MUST(ByteBuffer::copy(http_request.method()));
cached_response->status = response.status();
cached_response->url = http_request.current_url();
cached_response->current_age = UnixDateTime::now();
m_cache.set(http_request.current_url(), move(cached_response));
Expand Down Expand Up @@ -1459,10 +1462,6 @@ class CachePartition : public RefCounted<CachePartition> {
// FIXME: Implement must-understand cache directive
}

// FIXME: This is just for now, ad-hoc — not adhering to any particular spec.
if (response.status() == 301 || response.status() == 302 || response.status() == 303 || response.status() == 307 || response.status() == 308)
return false;

// - the no-store cache directive is not present in the response (see Section 5.2.2.5);
if (request.cache_mode() == Infrastructure::Request::CacheMode::NoStore)
return false;
Expand Down

0 comments on commit 7959c42

Please sign in to comment.