New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Make the WebSocket handshake ourselves 🍷 #16180
Conversation
Heads up! This PR modifies the following files:
|
r? @jdm |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Only tiny comments.
components/net/websocket_loader.rs
Outdated
let secure = match url.scheme() { | ||
"ws" => false, | ||
"wss" => true, | ||
_ => panic!("URL's scheme should be http or https"), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ws or wss?
components/net/websocket_loader.rs
Outdated
// Step 14.6. | ||
let protocol_in_use = if let Some(response_protocols) = response.headers.get::<WebSocketProtocol>() { | ||
for p in &**response_protocols { | ||
if !protocols.contains(p) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The previous check used eq_ignore_ascii_case. Is this the cause of the new test failure?
We need to switch to tungstenite to finally update openssl, this commit rewrites the whole websocket infrastructure to properly follow the Fetch spec and to make switching to a different websocket library easier.
@bors-servo: r+ |
📌 Commit d022535 has been approved by |
@bors-servo p=17 |
Make the WebSocket handshake ourselves 🍷 HYPE <!-- Reviewable:start --> --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/16180) <!-- Reviewable:end -->
☀️ Test successful - android, arm32, arm64, linux-dev, linux-rel-css, linux-rel-wpt, mac-dev-unit, mac-rel-css, mac-rel-wpt1, mac-rel-wpt2, windows-msvc-dev |
HYPE
This change is