Integrate fetch into WebSocket implementation#36616
Merged
yezhizhen merged 2 commits intoservo:mainfrom Oct 18, 2025
Merged
Conversation
a391b0c to
4d28c0a
Compare
4d28c0a to
4d9592a
Compare
9a5a2d6 to
ef6ed36
Compare
jdm
reviewed
Jul 2, 2025
051c265 to
84bebba
Compare
pylbrecht
commented
Jul 2, 2025
7be9a4f to
6ade97e
Compare
883dd29 to
7e9a409
Compare
26c3c14 to
f20136b
Compare
f20136b to
31e19ed
Compare
Member
|
I'm a big fan of the direction that the latest commit is going in. 🚀 |
a69555b to
57ad9a7
Compare
57ad9a7 to
e5f9da2
Compare
pylbrecht
commented
Sep 6, 2025
91d0b94 to
9ae2752
Compare
9ae2752 to
412d26a
Compare
pylbrecht
added a commit
to pylbrecht/servo
that referenced
this pull request
Oct 14, 2025
Resolving servo#36616 (comment). Signed-off-by: pylbrecht <pylbrecht@mailbox.org>
pylbrecht
added a commit
to pylbrecht/servo
that referenced
this pull request
Oct 14, 2025
Resolving servo#36616 (comment). Signed-off-by: pylbrecht <pylbrecht@mailbox.org>
pylbrecht
added a commit
to pylbrecht/servo
that referenced
this pull request
Oct 14, 2025
Resolving servo#36616 (comment). Signed-off-by: pylbrecht <pylbrecht@mailbox.org>
pylbrecht
added a commit
to pylbrecht/servo
that referenced
this pull request
Oct 15, 2025
Resolving servo#36616 (comment). Signed-off-by: pylbrecht <pylbrecht@mailbox.org>
pylbrecht
added a commit
to pylbrecht/servo
that referenced
this pull request
Oct 15, 2025
Resolving servo#36616 (comment). Signed-off-by: pylbrecht <pylbrecht@mailbox.org>
pylbrecht
added a commit
to pylbrecht/servo
that referenced
this pull request
Oct 15, 2025
Resolving servo#36616 (comment). Signed-off-by: pylbrecht <pylbrecht@mailbox.org>
fetch() into WebSocket implementation
fetch() into WebSocket implementation
jdm
approved these changes
Oct 17, 2025
Member
jdm
left a comment
There was a problem hiding this comment.
Let's get this cleaned up and merged!!
979fe24 to
737be28
Compare
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 * w3c/webappsec-csp#532 --- Fixes: servo#35028 --- Signed-off-by: pylbrecht <pylbrecht@mailbox.org>
737be28 to
948a742
Compare
Member
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

Description
For the initial WebSocket handshake, we need to create a proper handshake request and
fetch()it. Creating this handshake request begins inWebSocketMethods::Constructor(), which sends the partially preparedRequestBuilderover tocomponents/net/resource_thread.rs. There we handle the incomingCoreResourceMsg::Fetchmessage and finish creating the handshake request incomponents/net/websocket_loader.rs::create_handshake_request(). Finally we fetch the request by callingcomponents/net/fetch/methods.rs::fetch().fetch()eventually callshttp_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 usestungstenite, 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 introducedRequest::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
Fixes: #35028