If a request failed, or was stopped, do not attempt to write the cache
entry footer to disk. Note that at this point, the cache index will not
have been created, thus this entry will not be used in the future. We do
still delete any partial file on disk.
This serves as a more general fix for the issue addressed in commit
9f2ac14521.
HeaderList::delete involves a Vector::remove_all_matching internally.
So if an exempt header appeared again later in the header list, we would
be accessing the name string of the previously deleted header.
We now partition the HTTP disk cache based on the Vary response header.
If a cached response contains a Vary header, we look for each of the
header names in the outgoing HTTP request. The outgoing request must
match every header value in the original request for the cache entry
to be used; otherwise, a new request will be issued, and a separate
cache entry will be created.
Note that we must now defer creating the disk cache file itself until we
have received the response headers. The Vary key is computed from these
headers, and affects the partitioned disk cache file name.
There are further optimizations we can make here. If we have a Vary
mismatch, we could find the best candidate cached response and issue a
conditional HTTP request. The content server may then respond with an
HTTP 304 if the mismatched request headers are actually okay. But for
now, if we have a Vary mismatch, we issue an unconditional request as
a purely correctness-oriented patch.
We need to store request headers in order to handle Vary mismatches.
(Note we should also be using BLOB for header storage in sqlite, as they
are not necessarily UTF-8.)
If the cache mode is no-store, we must not interact with the cache at
all.
If the cache mode is reload, we must not use any cached response.
If the cache-mode is only-if-cached or force-cache, we are permitted
to respond with stale cache responses.
Note that we currently cannot test only-if-cached in test-web. Setting
this mode also requires setting the cors mode to same-origin, but our
http-test-server infra requires setting the cors mode to cors.
Once a cache entry is not fresh, we now remove it from the memory cache.
We will avoid handling revalidation from within WebContent. Instead, we
will just forward the request to RequestServer, where the disk cache
will handle revalidation for itself if needed.
We currently disable the disk cache because the WPT runner will run more
than one RequestServer process at a time. The SQLite database does not
handle this concurrent read/write access well.
We will now enable the disk cache with a per-process database. This is
needed to ensure that WPT Fetch cache tests are sufficiently handled by
RequestServer.
We currently set the response time to when the cache entry writer is
created. This is more or less the same as the request start time, so
this is not correct.
This was a regression from 5384f84550.
That commit changed when the writer was created, but did not move the
setting of the response time to match.
We now set the response time to when the HTTP response headers have been
received (again), which matches how Chromium behaves:
https://source.chromium.org/chromium/chromium/src/+/refs/tags/144.0.7500.0:net/url_request/url_request_job.cc;l=425-433
If we have the response for a non-Range request in the memory cache, we
would previously use it in reply to Range requests. Similar to commit
878b00ae61f998a26aad7f50fae66cf969878ad6, we are just punting on Range
requests in the HTTP caches for now.
A future commit will format memory cache debug messages similarly to the
disk cache messages. To make it easy to read them both at a glance when
both debug flags are turned on, let's add a prefix to these messages.
We currently do not handle responses for range requests at all in our
HTTP caches. This means if we issue a request for a range of bytes=1-10,
that response will be served to a subsequent request for a range of
bytes=10-20. This is obviously invalid - so until we handle these
requests, just don't cache them for now.
If the cURL request completes with anything other than CURLE_OK, we must
not keep the cache entry. For example, if the server's connection closes
while transferring data, we receive CURLE_PARTIAL_FILE. We don't want
this cache entry to be treated as valid in a subsequent request.
The in-memory HTTP Fetch cache currently keeps the realm which created
each cache entry alive indefinitely. This patch migrates this cache to
LibHTTP, to ensure it is completely unaware of any JS objects.
Now that we are not interacting with Fetch response objects, we can no
longer use Streams infrastructure to pipe the response body into the
Fetch response. Fetch also ultimately creates the cache response once
the HTTP response headers have arrived. So the LibHTTP cache will hold
entries in a pending list until we have received the entire response
body. Then it is moved to a completed list and may be used thereafter.
No need to duplicate this in LibWeb.
In doing so, this also fixes an apparent bug for SWR handling in LibWeb.
We were previously deciding if we were in the SWR lifetime with:
stale_while_revalidate > current_age
However, the SWR lifetime is meant to be an additional time on top of
the freshness lifetime:
freshness_lifetime + stale_while_revalidate > current_age
This directive allows our disk cache to serve stale responses for a time
indicated by the directive itself, while we revalidate the response in
the background.
Issuing requests that weren't initiated by a client is a new thing for
RequestServer. In this implementation, we associate the request with
the client that initiated the request to the stale cache entry. This
adds a "background request" mode to the Request object, to prevent us
from trying to send any of the revalidation response over IPC.
We were returning the incorrect result when upgrading a cache entry to
have exclusivity on must-revalidate requests. This could result in the
entry being read and updated at the same time, especially if the server
returned a non-304 response.
There are a couple of remaining RFC 9111 methods in LibWeb's Fetch, but
these are currently directly tied to the way we store GC-allocated HTTP
response objects. So de-coupling that is left as a future exercise.
We currently have two ongoing implementations of RFC 9111, HTTP caching.
In order to consolidate these, this patch moves the implementation from
RequestServer to LibHTTP for re-use within LibWeb.
The end goal here is for LibHTTP to be the home of our RFC 9111 (HTTP
caching) implementation. We currently have one implementation in LibWeb
for our in-memory cache and another in RequestServer for our disk cache.
The implementations both largely revolve around interacting with HTTP
headers. But in LibWeb, we are using Fetch's header infra, and in RS we
are using are home-grown header infra from LibHTTP.
So to give these a common denominator, this patch replaces the LibHTTP
implementation with Fetch's infra. Our existing LibHTTP implementation
was not particularly compliant with any spec, so this at least gives us
a standards-based common implementation.
This migration also required moving a handful of other Fetch AOs over
to LibHTTP. (It turns out these AOs were all from the Fetch/Infra/HTTP
folder, so perhaps it makes sense for LibHTTP to be the implementation
of that entire set of facilities.)
The only thing in HTTPResponse being used is reason_phrase_for_code,
which is just a static helper method. Move it to its own file and remove
HTTPResponse.
This is just one less thing to have to port to an upcoming HTTP header
refactor.
C++ will jovially select the implicit conversion operator, even if it's
complete bogus, such as for unknown-size types or non-destructible
types. Therefore, all such conversions (which incur a copy) must
(unfortunately) be explicit so that non-copyable types continue to work.
NOTE: We make an exception for trivially copyable types, since they
are, well, trivially copyable.
Co-authored-by: kleines Filmröllchen <filmroellchen@serenityos.org>
These changes are arbitrarily divided into multiple commits to make it
easier to find potentially introduced bugs with git bisect.Everything:
The modifications in this commit were automatically made using the
following command:
find . -name '*.cpp' -exec sed -i -E 's/dbg\(\) << ("[^"{]*");/dbgln\(\1\);/' {} \;
This patchset makes ProtocolServer stream the downloads to its client
(LibProtocol), and as such changes the download API; a possible
download lifecycle could be as such:
notation = client->server:'>', server->client:'<', pipe activity:'*'
```
> StartDownload(GET, url, headers, {})
< Response(0, fd 8)
* {data, 1024b}
< HeadersBecameAvailable(0, response_headers, 200)
< DownloadProgress(0, 4K, 1024)
* {data, 1024b}
* {data, 1024b}
< DownloadProgress(0, 4K, 2048)
* {data, 1024b}
< DownloadProgress(0, 4K, 1024)
< DownloadFinished(0, true, 4K)
```
Since managing the received file descriptor is a pain, LibProtocol
implements `Download::stream_into(OutputStream)`, which can be used to
stream the download into any given output stream (be it a file, or
memory, or writing stuff with a delay, etc.).
Also, as some of the users of this API require all the downloaded data
upfront, LibProtocol also implements `set_should_buffer_all_input()`,
which causes the download instance to buffer all the data until the
download is complete, and to call the `on_buffered_download_finish`
hook.
Problem:
- `(void)` simply casts the expression to void. This is understood to
indicate that it is ignored, but this is really a compiler trick to
get the compiler to not generate a warning.
Solution:
- Use the `[[maybe_unused]]` attribute to indicate the value is unused.
Note:
- Functions taking a `(void)` argument list have also been changed to
`()` because this is not needed and shows up in the same grep
command.
Almost everyone using this API actually wanted String instead of a
ByteBuffer anyway, and there were a bunch of slightly different ways
clients would convert to String.
Let's just cut out all the confusion and make it return String. :^)
Now we (almost) verify all the sites we browse.
Certificate verification failures should not be unexpected, as the
existing CA certificates are likely not complete.
