This ensures that the controller GCPtr is non-null by the time we
capture a copy of it for the lambda passed to the request signal's
add_abort_algorithm() method. Currently, the VERIFY() would always fail
when aborting the signal. The alternative to this would be adding a cell
setter to Handle, and ensuring that null handles have a HandleImpl as
well; this seems not worth the hassle right now. Thanks to Lubrsi for
catching this!
Co-authored-by: Luke Wilde <lukew@serenityos.org>
We were accidentally copying these from the newly created Request
object's underlying request, to itself. Thanks to Lubrsi for catching
this!
Co-authored-by: Luke Wilde <lukew@serenityos.org>
This was an oversight from when I converted PendingResponse and various
other classes from being ref-counted to GC-allocated last minute - no
one takes care to keep all of them alive. Some are on the stack, and
some might be captured in another PendingResponse's JS::SafeFunction,
but ultimately, we need a better solution.
Since a PendingResponse is *always* the result of someone having created
a Request, let's just let that keep a list of each PendingResponse that
has been created for it, and visit them until they are resolved. After
that, they can be GC'd with no complaints.
With so much infrastructure implemented, we can finally add the last
piece of this puzzle - the fetch() method itself!
This contains a few hundred lines of generated code as handling the
RequestInfo and RequestInfo parameter types manually is not feasible,
but we can't use the IDL definition as the Window object is handwritten
code at the moment.
It's neatly tucked away in Bindings/ and will be removed eventually.
This implements the following operations from section 4 of the Fetch
spec (https://fetch.spec.whatwg.org/#fetching):
- Fetch
- Main fetch
- Fetch response handover
- Scheme fetch
- HTTP fetch
- HTTP-redirect fetch
- HTTP-network-or-cache fetch (without caching)
It does *not* implement:
- HTTP-network fetch
- CORS-preflight fetch
Instead, we let ResourceLoader handle the actual networking for now,
which isn't ideal, but certainly enough to get enough functionality up
and running for most websites to not complain.
There will be a lot of different cases where we'll return an error
response, and having a customized Promise rejection message seems quite
useful.
Note that this has to be distinct from the existing 'status message',
which is required to be empty in those cases.
The header-specific ABNF rules are completely ignored for now, but we
can at least extract a single header value, which at least works for
simple cases like `Location`-based redirects.
This is the way.
On a more serious note, there's no reason to keep adding ref-counted
classes to LibWeb now that the majority of classes is GC'd - it only
adds the risk of discovering some cycle down the line, and forces us to
use handles as we can't visit().
We need to keep an Infrastructure::Request::BodyType around as we're not
sure what's actually inside, instead accessing Infrastructure::Body
directly.
Co-authored-by: Luke Wilde <lukew@serenityos.org>
This allows us to use this:
```cpp
auto header = TRY_OR_RETURN_OOM(realm,
Infrastructure::Header::from_string_pair(name, value));
```
Instead of the somewhat unwieldly:
```cpp
auto header = Infrastructure::Header {
.name = TRY_OR_RETURN_OOM(realm, ByteBuffer::copy(name.bytes())),
.value = TRY_OR_RETURN_OOM(realm, ByteBuffer::copy(value.bytes())),
};
```
In particular, StringView::contains(char) is often used with a u32
code point. When this is done, the compiler will for some reason allow
data corruption to occur silently.
In fact, this is one of two reasons for the following OSS Fuzz issue:
https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=49184
This is probably a very old bug.
In the particular case of URLParser, AK::is_url_code_point got confused:
return /* ... */ || "!$&'()*+,-./:;=?@_~"sv.contains(code_point);
If code_point is a large code point that happens to have the correct
lower bytes, AK::is_url_code_point is then convinced that the given
code point is okay, even if it is actually problematic.
This commit fixes *only* the silent data corruption due to the erroneous
conversion, and does not fully resolve OSS-Fuzz#49184.
With the addition of the 'fetch params' struct, the single ownership
model we had so far falls apart completely.
Additionally, this works nicely for FilteredResponse's internal response
instead of risking a dangling reference.
Replacing the public constructor with a create() function also found a
few instances of a Request being stack-allocated!
