Writing `termination_reason` in the closure in `http_network_fetch()`
had no effect, as the response is captured by value, and cannot be
captured by mutable reference as it is needed later in the outer
function.
This is flagged as a warning with Rust 1.92:
https://github.com/servo/servo/actions/runs/21750902723/job/62748597306?pr=42402
Note: I don't understand this code deeply, I was just trying my best to
fix the warning. I'm more than happy to close this PR or change the code
if someone more familiar has a better idea of how to fix it.
Testing: Existing tests, no behaviour change.
Signed-off-by: Alice Boxhall <alice@igalia.com>
This switches FileManager from being behind an Arc<Mutex<FileManager>>>
to just FileManager.
This saves us the Arc and Mutex and the compiler makes sure that we do
not have Race Conditions.
FileManager is already easily cloneable because it consist of store:
Arc<> and GenericEmbedderProxy.
Testing: Compilation and unit tests are the tests.
Signed-off-by: Narfinger <Narfinger@users.noreply.github.com>
This PR fixes error handling in Servo's HTTP fetch implementations to
properly handle content decompression failures (e.g., bad gzip content)
by treating them as network errors as defined in WPT.
- Fixed some previously failing tests in bad-gzip-body.any.js that now
properly reject when consuming bodies with bad gzip content
- Several fetch metadata tests now pass correctly
---------
Signed-off-by: araya <araya@araya.dev>
Regardless of the `Ok` status, we should always create a
stylesheet. However, the determining of the load/error event
is based on whether it has parsed actual content or not.
It also realigns some of the spec text to now only do it for link
elements, since for styles we shouldn't be checking the result of
the content type.
Part of #22715
---------
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: Martin Robinson <mrobinson@igalia.com>
Devtools Channel does not need mutability. Hence, we do not need an
Arc<Mutex<_>>.
As the underlying channel is a Crossbeam Sender the clone should be
relatively cheap.
Signed-off-by: Narfinger <Narfinger@users.noreply.github.com>
Testing: Thanks to rust compilation means correctness.
---------
Signed-off-by: Narfinger <Narfinger@users.noreply.github.com>
#41857 exposed some more code in the net crate that performed blocking
communication with the embedder and could prevent other networking tasks
from running. To address that, we need the net code to perform async
receive operations, which requires passing tokio channels to the
embedder.
However, the current embedding message design puts all messages in the
same enum and requires that they are serializable. Embedder messages
from the network do not require this property, since they run in the
same process as the embedder. Therefore, this PR creates a new
EmbedderProxy structure that is generic over the message, allowing each
component of Servo to use a embedder message type that is specific to
that component.
The final benefit of this set of changes is that the embedder messages
for a particular crate can now live in the crate itself (since the only
crate that depends on it is the servo crate), not in the shared
`embedding_traits` crate. This hugely reduces the amount of code that
needs to be rebuilt when changing these messages, enabling much faster
incremental builds for those changes.
Testing: Strictly refactoring; existing test coverage is sufficient.
Fixes: #41958
---------
Signed-off-by: Josh Matthews <josh@joshmatthews.net>
This updates the existing algorithm to handle three
cases rather than the previous two. The spec was updated
afterwards and wasn't reflected in the code yet.
The usage of `response.redirect_taint` is in [1],
but we don't implement that quite right either. However,
postponing that to a future PR to keep things manageable.
Part of #41807
[1]: https://fetch.spec.whatwg.org/#fetch-finale
Signed-off-by: Tim van der Lippe <tvanderlippe@gmail.com>
Rather than this catch-all (which I copy-pasted from a usage where I
should have introduced a specific enum value), we now should always use
one of the enum values. If one does not exist, then it should be added
as such.
Follow-up for #36434
Testing: it builds
Signed-off-by: Tim van der Lippe <tvanderlippe@gmail.com>
These keep-alive records live on the `CoreResourceManager` since the vec
of records must be modified by the fetch thread. The script thread
sometimes
requires this information as well, which is why it can send a message to
obtain the total size.
The keep-alive records must be tracked per global. That's why all code
needs
to specify the `pipeline_id`. Requests optionally have this field, which
is why
the code expects it to be present. The relevant information is added to
the
navigator request to ensure it can compute it.
Fixes#41230
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>
First PR for #40232.
Implement the getSecurityInfo protocol message to allow DevTools clients
to inspect TLS connection details for HTTPS requests, including:
- Protocol version (TLS 1.2/1.3)
- Cipher suite
- Key exchange group
- ALPN protocol
- HSTS and ECH status
- Security state
Security state is simplified (secure/insecure only) and certificate
details are stubbed out; both will be completed in follow-up work.
Signed-off-by: jiang1997 <jieke@live.cn>
This changes the certificate verifier to use rustls_platform_verifier
under the hood.
**Note**: The rustls-platform-verifier has specific instructions for
android and currently we fall back to the webpki roots for this
platform.
Testing: This was tested on linux, macos and ohos and the rest in the
CI.
Fixes: Should fix https://github.com/servo/servo/issues/32903 and
partially https://github.com/servo/servo/issues/35227
---------
Signed-off-by: Narfinger <Narfinger@users.noreply.github.com>
This implements a dummy `mailto:` protocol handler in
Servoshell. It registers a new custom handler that
fetches content by substituting the registered url
for the protocol handler.
This does not pass any WPT tests, as that requires
using the `document.protocol_handler_automation_mode`
which will be part of a follow-up PR.
Part of #40615
Signed-off-by: Tim van der Lippe <tvanderlippe@gmail.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>
# Description
For the initial WebSocket handshake, we need to create a proper
handshake request and `fetch()` it. Creating this handshake request
begins in `WebSocketMethods::Constructor()`, which sends the partially
prepared `RequestBuilder` over to `components/net/resource_thread.rs`.
There we handle the incoming `CoreResourceMsg::Fetch` message and finish
creating the handshake request in
`components/net/websocket_loader.rs::create_handshake_request()`.
Finally we fetch the request by calling
`components/net/fetch/methods.rs::fetch()`.
`fetch()` eventually calls `http_network_fetch()`. This is where the
"actual fetching" of the request takes place on a network level, which
means we need to handle WebSocket handshake requests differently than
non-WebSocket requests. I mostly moved the existing code, which uses
`tungstenite`, with some type massaging (thanks again, @jdm, for helping
me out here!). This included converting from tungstenite types to
Servo's net types (request/response).
# The tricky bits
In order to fetch the handshake request via
`components/net/fetch/methods.rs::fetch()`, we need to convert the
request URL's scheme from ws(s) to http(s). Then, we need to "undo" this
conversion again when doing CSP checks (i.e. http(s) back to ws(s)). To
avoid having this "undoing" logic in a bunch of places we introduced
`Request::original_url()`, holding the URL before the scheme conversion
took place. Unfortunately this only gets as so far. There are still some
places, where we need to explicitly check and/or convert the URL scheme
(e.g. retroactively upgrading to a secure scheme).
# Related
* https://websockets.spec.whatwg.org/#concept-websocket-establish
* https://fetch.spec.whatwg.org/#http-network-fetch
* https://github.com/w3c/webappsec-csp/issues/532
---
Fixes: #35028
---------
Signed-off-by: pylbrecht <pylbrecht@mailbox.org>
On fetching of the request's current URL with some URL schemes ("about",
"data") the response body could be empty and UA can silently skip chunk
processing and don't send empty data over IPC to embeddee.
See https://fetch.spec.whatwg.org/#concept-scheme-fetch
- "about:blank" -> empty body
- "data:," -> empty body
Testing: This is performance enhancement for the internal closed clients
(HTML `image/media/script/video` elements) and public DOM `Fetch` API,
and no observable WPT expectation changes because DOM `Response +
ReadableStream` are processing empty fetched chunk (from "about:blank",
"data:,") without any trackable state changes.
Signed-off-by: Andrei Volykhin <andrei.volykhin@gmail.com>
The current behaviour in dev tools network monitor is missing data for
the `Transferred` size and `Content` size.
We currently have a fn `response_content` that constructs the
`ResponseContentMsg` struct where the two missing data fields are
defined and set to zero values.
These current changes calculates the data in the `response_content` fn
and sends a `NetworkEvent::HttpResponse` to the client when the final
body is done.
Currently, we have data appearing in the `Transferred` column of the
network panel
fixes: https://github.com/servo/servo/issues/38126
---------
Signed-off-by: uthmaniv <uthmanyahayababa@gmail.com>
This change refactors how we notify DevTools about network activity so
that all fetches (even those served from cache) appear correctly in the
Network panel, and so that DevTools sees request metadata as soon as
possible rather than waiting until the end of a full HTTP cycle.
- Before, we only send DevTools events inside http_network_fetch, so
cached responses (which skip that path) never show up. By emitting a
minimal HttpRequest event at the very start of main_fetch (with URL,
method, pipeline and browsing IDs), we guarantee every fetch shows up
immediately.
- Then, by moving HttpResponse notifications out of http_network_fetch
into main_fetch (right after process_response and process_response_eof),
we ensure DevTools gets status, header, and completion events for both
network and cache hits. Leveraging nullable fields in NetworkEventActor
lets us incrementally fill in timing, header, and body data later,
improving DevTools’ visibility.
Testing: Ran servo with `--devtools=6080` flag, cached responses now
appear in the network panel
Fixes: https://github.com/servo/servo/issues/37869
---------
Signed-off-by: Uthman Yahaya Baba <uthmanyahayababa@gmail.com>
This changes includes two semi-related things:
1. Fixes some specification compliance issues when parsing mime
types and charsets for `XMLHttpRequest`.
2. Implements a `<stylesheet>` parsing quirk involving mime types.
Testing: There are tests for these changes.
Signed-off-by: Martin Robinson <mrobinson@igalia.com>
Co-authored-by: Martin Robinson <mrobinson@igalia.com>
This turned out to be a full rabbit hole. The new header
is parsed in the new `parse_csp_list_from_metadata` which
sets `disposition` to `report.
I was testing this with
`script-src-report-only-policy-works-with-external-hash-policy.html`
which was blocking the script incorrectly. Turns out that there
were multiple bugs in the CSP library, as well as a missing
check in `fetch` to report violations.
Additionally, in several locations we were manually reporting csp
violations, instead of the new `global.report_csp_violations`. As
a result of that, they would double report, since the report-only
header would be appended as a policy and now would report twice.
Now, all callsides use `global.report_csp_violations`. As a nice
side-effect, I added the code to set source file information,
since that was already present for the `eval` check, but nowhere
else.
Part of #36437
Requires servo/rust-content-security-policy#5
---------
Signed-off-by: Tim van der Lippe <tvanderlippe@gmail.com>
Signed-off-by: Tim van der Lippe <TimvdLippe@users.noreply.github.com>
It also updates the FetchResponseListener to process CSP violations to
ensure that iframe elements (amongst others) properly generate the CSP
events. These iframe elements are used in the Trusted Types tests
themselves and weren't propagating the violations before.
However, the tests themselves are still not passing since they also use
Websockets, which currently aren't using the fetch machinery itself.
That is fixed as part of [1].
[1]: https://github.com/servo/servo/issues/35028
---------
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>
Propagate through documents a flag that represents if any of the
ancestor navigables has a potentially trustworthy origin.
The "potentially trustworthy origin" concept appears to have gotten
confused in a couple of places and we were instead testing if a URL had
"potentially trustworthy" properties.
The main test for the ancestor navigables is
[mixed-content/nested-iframes](https://github.com/web-platform-tests/wpt/blob/master/mixed-content/nested-iframes.window.js)
---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by
`[X]` when the step is complete, and replace `___` with appropriate
data: -->
- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [X] These changes fix#36108
<!-- Either: -->
- [X] There are tests for these changes
---------
Signed-off-by: Sebastian C <sebsebmc@gmail.com>
- Remove `EmbedderMethods::get_user_agent_string`. This is now part of
the `Preferences` data structure, which should allow it to be
per-`WebView` in the future.
- Remove `EmbedderMethods::get_version_string`. This was used to include
some data along with WebRender captures about the Servo version. This
isn't really necessary and it was done to replace code in the past
that output the WebRender version, so also isn't what the original
code did. I think we can just remove this entirely.
The idea with these changes is that `EmbedderMethods` can be removed
in a followup and the rest of the methods can be added to
`ServoDelegate`. These two methods are ones that cannot be added to a
delegate as they are used during `Servo` initialization.
Testing: There is currently no testing for libservo. These changes are
meant
as preparation for adding a suite of `WebView` unit tests.
Signed-off-by: Martin Robinson <mrobinson@igalia.com>
Signed-off-by: Martin Robinson <mrobinson@igalia.com>
* Add doc comments to RequestBuilder fields/methods
Signed-off-by: Simon Wülker <simon.wuelker@arcor.de>
* Implement Request::cryptographic_nonce_metadata
Signed-off-by: Simon Wülker <simon.wuelker@arcor.de>
* Implement HTMLOrSVGElement::nonce
Signed-off-by: Simon Wülker <simon.wuelker@arcor.de>
* Set request cryptographic nonce metadata for link elements
Signed-off-by: Simon Wülker <simon.wuelker@arcor.de>
* Set request's cryptographic nonce when fetching scripts
Signed-off-by: Simon Wülker <simon.wuelker@arcor.de>
* Forward request nonce to rust-content-security-policy
Signed-off-by: Simon Wülker <simon.wuelker@arcor.de>
* Update WPT expectations
Signed-off-by: Simon Wülker <simon.wuelker@arcor.de>
---------
Signed-off-by: Simon Wülker <simon.wuelker@arcor.de>
This patch exposes a servo internal DOM API that is only made available to about:
pages on the navigator object to request memory reports. The about:memory page itself is
loaded like other html resources (eg. bad cert, net error) and makes use of this new API.
On the implementation side, notable changes:
- components/script/routed_promise.rs abstracts the setup used to fulfill a promise when the
work needs to be routed through the constellation. The goal is to migrate other similar
promise APIs in followup (eg. dom/webgpu/gpu.rs, bluetooth.rs).
- a new message is added to request a report from the memory reporter, and the memory reporter
creates a json representation of the set of memory reports.
- the post-processing of memory reports is done in Javascript in the about-memory.html page,
providing the same results as the current Rust code that outputs to stdout. We can decide
later if we want to remove the current output.
Signed-off-by: webbeef <me@webbeef.org>
Rework the `WebViewDelegate::intercept_web_resource_load` into
`WebViewDelegate::load_web_resource` and clean up internal messaging.
The main thing here is adding objects which manage the response to these
delegate methods. Now we have `WebResourceLoad` and
`InterceptedWebResourceLoad` which make it much harder to misuse the
API.
In addition, the internal messaging for this is cleaned up. Canceling
and finishing the load are unrelated to the HTTP body so they are no
longer subtypes of an HttpBodyData message. Processing of messages is
made a bit more efficient by collecting all body chunks in a vector and
only flattening the chunks at the end.
Finally, "interceptor" is a much more common spelling than "intercepter"
so I've gone ahead and made this change everywhere.
Signed-off-by: Martin Robinson <mrobinson@igalia.com>
Blocking a fetch due to a bad port should be grouped together
with CSP blocks as per the spec, but these steps were previously
seperated.
Additionally, remove handling of ftp in
should_request_be_blocked_due_to_a_bad_port, since it did nothing
anyways.
Signed-off-by: Simon Wülker <simon.wuelker@arcor.de>
`EmbedderMsg` was previously paired with an implicit
`Option<WebViewId>`, even though almost all variants were either always
`Some` or always `None`, depending on whether there was a `WebView
involved.
This patch adds the `WebViewId` to as many `EmbedderMsg` variants as
possible, so we can call their associated `WebView` delegate methods
without needing to check and unwrap the `Option`. In many cases, this
required more changes to plumb through the `WebViewId`.
Notably, all `Request`s now explicitly need a `WebView` or not, in order
to ensure that it is passed when appropriate.
Signed-off-by: Delan Azabani <dazabani@igalia.com>
Co-authored-by: Martin Robinson <mrobinson@igalia.com>
Instead of creating an IPC channel for every fetch, allow cancelling
fetches based on the `RequestId` of the original request. This requires
that `RequestId`s be UUIDs so that they are unique between processes
that might communicating with the resource process.
In addition, the resource process loop now keeps a `HashMap` or `Weak`
handles to cancellers and cleans them up.
This allows for creating mutiple `FetchCanceller`s in `script` for a
single fetch request, allowing integration of the media and video
elements to integrate with the `Document` canceller list -- meaning
these fetches also get cancelled when the `Document` unloads.
Signed-off-by: Martin Robinson <mrobinson@igalia.com>
This allows reusing the asynchrnous fetch mechanism that we use for page
resources and is likely a step toward removing the `FetchThread`.
Benefits:
- Reduces IPC traffic during navigation. Now instead of bouncing
between the constellation and the `ScriptThread` responses are sent
directly to the `ScriptThread`.
- Allows cancelling loads after redirects, which was not possible
before.
There is the question of what to do when a redirect is cross-origin
(#23037). This currently isn't handled properly as the `Constellation`
sends data to the same `Pipeline` that initiated the load. This change
doesn't fix this issue, but does make it more possible for the
`ScriptThread` to shut down the pipeline and ask the `Constellation` to
replace it with a new one.
Signed-off-by: Martin Robinson <mrobinson@igalia.com>
