`blob` URLs have a implicit blob URL entry attached, which stores the
data contained in the blob. The specification requires this entry to be
resolved as the URL is parsed. We only resolve it inside `net` when
loading the URL. That causes problems if the blob entry has been revoked
in the meantime - see https://github.com/servo/servo/issues/25226.
Ideally we would want to resolve blobs at parse-time as required. But
because `ServoUrl` is such a fundamental type, I've not managed to do
this change without having to touch hundreds of files at once.
Thus, we now require passing a `UrlWithBlobClaim` instead of a
`ServoUrl` when `fetch`-ing. This type proves that the caller has
acquired the blob beforehand.
As a temporary escape hatch, I've added
`UrlWithBlobClaim::from_url_without_having_claimed_blob`. That method
logs a warning if its used unsafely. This method is currently used in
most places to keep this change small. Only workers now acquire the blob
beforehand.
Testing: A new test starts to pass
Part of https://github.com/servo/servo/issues/43326
Part of https://github.com/servo/servo/issues/25226
---------
Signed-off-by: Simon Wülker <simon.wuelker@arcor.de>
Co-authored-by: Josh Matthews <josh@joshmatthews.net>
Follow up of #43526. This addresses Nico's comment:
https://github.com/servo/servo/pull/43526#issuecomment-4104953308
- `bluetooth_traits` -> `servo_bluetooth_traits`
- `base` -> `servo_base`
- `bluetooth` -> `servo_bluetooth`
- `background_hang_monitor` -> `servo_background_hang_monitor`
Testing: This should not change any behaviour.
---------
Signed-off-by: Euclid Ye <yezhizhenjiakang@gmail.com>
This is no longer present in the spec. Instead, the
`process_request_body` is the new way. These two
methods were called right after each other and there was only 1
implementation in `htmlvideoelement`. That implementation is now moved
to `process_request_body` and hence we can remove the unnecessary
method.
Testing: It compiles
Signed-off-by: Tim van der Lippe <tvanderlippe@gmail.com>
Add the cx parameter to `fn process_response` in the
`FetchResponseListener` trait and the traits that that interface change
requires. Chose to add it as the first parameter, following the same
order that `FetchResponseListener::process_response_eof` uses.
Testing: Checked that servo builds locally as well as `./mach fmt` and
`./mach test-tidy`. I don't think more tests are needed as we are not
introducing new functionality
Fixes: #42840
---------
Signed-off-by: Javier Olaechea <pirata@gmail.com>
This PR fixes two related issues with Content Security Policy (CSP)
nonce validation for external scripts:
1. Missing nonce validation for external scripts with malformed
attributes
2. Incorrect violation event reporting for blocked external resources
This makes servo closer to passing the `nonce-enforce-blocked` wpt test.
The remaining failures are blocked by required changes in the html
parser.
1. Svg script support (https://github.com/servo/html5ever/issues/118)
```html
<svg xmlns="http://www.w3.org/2000/svg">
<script attribute attribute nonce="abc">
t.unreached_func("Duplicate attribute in SVG, no execution.")();
</script>
</svg>
```
2. Duplicate attrs check
the html parser needs to provide this flag, as mentioned on the original
commit message
(4821bc0ab0)
```html
<script attribute attribute nonce="abc">
t.unreached_func("Duplicate attribute, no execution.")();
</script>
<script attribute attribute=<style nonce="abc">
t.unreached_func("2# Duplicate attribute, no execution.")();
</script>
[...]
<script src="../support/nonce-should-be-blocked.js?5" attribute attribute nonce="abc"></script>
```
I've also created a PR to implement the duplicate attrs flag on
html5ever https://github.com/servo/html5ever/pull/695
Testing: doesn't fixes the aforementioned wpt test yet.
Fixes: part of #36437
---------
Signed-off-by: Dyego Aurélio <dyegoaurelio@gmail.com>
`process_response_eof` is the only method that needs cx at least right
now. This PR removes one temp_cx and introduces one, removing that one
will is hard (needs VirtualMethods and a lot of work)
Testing: Just refactor
Part of #40600
---------
Signed-off-by: sagudev <16504129+sagudev@users.noreply.github.com>
Rather than having each callside specifying the relevant
information from the GlobalScope, do this via a trait instead.
This would have saved us quite a bit of test debugging
since we would often forget to set relevant information
from the global context for a request.
Now, in the future when we need additional information from
the globalscope for a request, we only need to update this
method to make that happen.
Previously it would also sometimes use `document`, but
calling the relevant information on either `document` or
`globalscope` doesn't matter, since the `globalscope`
defers to the value from the `document` anyways.
Signed-off-by: Tim van der Lippe <tvanderlippe@gmail.com>
This implements waiting for pending preloads, where the preload request
is still fetching the result when the second "real" request is started.
It is
implemented by storing responses in the `SharedPreloadedResources`
which is communicated via `PreloadId` send to the `CoreResourceManager`.
Part of #35035
---------
Signed-off-by: Tim van der Lippe <tvanderlippe@gmail.com>
Signed-off-by: Josh Matthews <josh@joshmatthews.net>
Co-authored-by: Josh Matthews <josh@joshmatthews.net>
Replace NetworkError::Internal with structured enum variants
- Adds
UnsupportedScheme,CorsViolation,ConnectionFailure,Timeout,RedirectError,InvalidMethod,ResourceError,SecurityBlock,MixedContent,CacheError,InvalidPort,
LocalDirectoryError, variants in NetworkError enum.
- Refactored the usage of NetworkError::Internal(String) to use the
appropriate new variant
Testing: Changes does not require test.
Fixes: https://github.com/servo/servo/issues/36434
---------
Signed-off-by: Uthman Yahaya Baba <uthmanyahayababa@gmail.com>
Signed-off-by: Usman Yahaya Baba <91813795+uthmaniv@users.noreply.github.com>
Signed-off-by: Tim van der Lippe <tvanderlippe@gmail.com>
Co-authored-by: Tim van der Lippe <tvanderlippe@gmail.com>
In preparation for adding preload support for all requests, we need to
add relevant client information to all these entrypoints. Additionally,
for links we now also set the referrer correctly and initialize
documents. All of which are required to start loading preload
information when processing requests.
Part of #35035
Signed-off-by: Tim van der Lippe <tvanderlippe@gmail.com>
Co-authored-by: Josh Matthews <josh@joshmatthews.net>
The `scripts_traits` crate was the only crate depending on `strum` with
the
`derive` feature. This accidentally allowed other crates to import strum
macros via `strum::` without declaring their own dependency on
`strum_macros`,
causing compilation issues when running `./mach test-unit -p net`.
This PR makes the imports consistent across the code base by:
- replacing all `strum_macro::` imports with `strum::` imports
- removing strum_macro dependencies
- adding derive feature to the strum workspace
Testing: Unit tests continue to pass
Signed-off-by: Jan Varga <jvarga@igalia.com>
The goal of this change is to prevent having to copy so much data out of
listeners when a fetch completes, which will be particularly important
for off-the-main thread parsing of CSS (see #22478). This change has
pros and cons:
Pros:
- This makes the design of the `FetchResponseListener` a great deal
simpler.
They no longer individually store a dummy `ResourceFetchTiming` that is
only replaced right before `process_response_eof`.
- The creation of the `Arc<Mutex<FetchResponseListener>>` in the
`NetworkListener` is abstracted away from clients and now they just
pass the `FetchResponseListener` to the fetch methods in the global.
Cons:
- Now each `FetchResponseListener` must explicitly call `submit_timing`
instead of having the `NetworkListener` do it. This is arguably a bit
easier to follow in the code.
- Since the internal data of the `NetworkListener` is now an
`Arc<Mutex<Option<FetchResponseListener>>>`, when the fetching code
needs to share state with the `NetworkListener` it either needs to
share an `Option` or some sort of internal state. In one case I've
stored the `Option` and in another case, I've stored a new inner
shared value.
Testing: This should not change observable behavior and is thus covered
by existing tests.
Fixes: #22550
---------
Signed-off-by: Martin Robinson <mrobinson@igalia.com>
`FetchReponseListener` has traditionally lived in `net` even though it
is only used in `script` currently. Because of the two way dependency,
it has also use a lot of templating to implement something pretty basic
(call methods on a trait object).
This change moves the trait to `script` and removes several levels of
templating, making the code quite a bit shorter and easier to
understand.
This change is preparation for fixing #22550 and implementing
off-the-main-thread CSS parsing.
Testing: This should not change any behavior so is covered by existing
tests.
Signed-off-by: Martin Robinson <mrobinson@igalia.com>
Moves interfaces defined by the performance spec to the
`script/dom/performance/` module from `script/dom/`.
Testing: Just a refactor shouldn't need any testing
Fixes: Partially #38901
Signed-off-by: WaterWhisperer <waterwhisperer24@qq.com>
This aligns the request object more with the specification,
since the spec now has a `traversable_for_user_prompts` and
a separate field for the client. Before, they were present
in the same enum.
In doing so, new structs are added that are all required in
the new spec. With this we can add support for preloaded
resources in this client, which are only populated when
we have an applicable Global.
Since the spec moved things around a bit, it now has a
dedicated method to populate the client from the request.
Unfortunately none of the WPT preload tests pass, since
the requests are received out-of-order. The specification
requires us to wait for that to settle, but I haven't figured
out yet how to do that. Given that this PR is already quite
large, opted to do that in a follow-up.
Part of #35035
Signed-off-by: Tim van der Lippe <tvanderlippe@gmail.com>
This was missed during the previous implementation and was the reason
that the CSP tests weren't working.
It also updates a test to ensure that audio and video are not preloaded.
No browsers do that and with this fix, the test now passes in Chrome. In
Firefox it still fails as it doesn't implement `.vtt` support.
Part of #35035
Signed-off-by: Tim van der Lippe <tvanderlippe@gmail.com>
The Link HTTP header can do the same as link elements,
in that they can preload/prefetch/etc... This implements
the basics of header parsing and hooks it up for preload.
Note that we use a new nom-rfc8288 crate that implements
the parsing behavior. However, that crate is too strict
in that empty attributes (;; as part of the header) are
discarded and resulting in a parsing failure. Therefore,
we use its lenient parsing mode.
Part of #35035
---------
Signed-off-by: Tim van der Lippe <tvanderlippe@gmail.com>
Signed-off-by: Tim van der Lippe <TimvdLippe@users.noreply.github.com>
Co-authored-by: Josh Matthews <josh@joshmatthews.net>
