Skip to content

Commit

Permalink
fix(websocket-websys): change close code in drop implementation to 1000
Browse files Browse the repository at this point in the history
In browsers, only the code 1000 and codes between 3000-4999 are allowed to be set by userland code: https://websockets.spec.whatwg.org/#dom-websocket-close

I found this while debugging use-after-free errors in our WASM application. Turns out, when connections are timing out, libp2p in rust drops them, but does not seem to close them explicitly (we are using libp2p-swarm). This led to these WebSockets still emitting events, the handlers of which were already dropped on the rust side, though.

A long investigation led me to have a look into the `Result` that gets returned from `close_with_code_and_reason`, and it turns out it's an error! Specifically:
```
InvalidAccessError: Failed to execute 'close' on 'WebSocket': The code must be either 1000, or between 3000 and 4999. 1001 is neither.
```

This PR only fixes the failing closing of the WebSocket, not the remaining use-after-free problem.

Pull-Request: #5229.
  • Loading branch information
sisou authored Mar 25, 2024
1 parent b890b29 commit 0687385
Show file tree
Hide file tree
Showing 5 changed files with 13 additions and 5 deletions.
2 changes: 1 addition & 1 deletion Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ libp2p-webrtc = { version = "0.7.1-alpha", path = "transports/webrtc" }
libp2p-webrtc-utils = { version = "0.2.0", path = "misc/webrtc-utils" }
libp2p-webrtc-websys = { version = "0.3.0-alpha", path = "transports/webrtc-websys" }
libp2p-websocket = { version = "0.43.0", path = "transports/websocket" }
libp2p-websocket-websys = { version = "0.3.1", path = "transports/websocket-websys" }
libp2p-websocket-websys = { version = "0.3.2", path = "transports/websocket-websys" }
libp2p-webtransport-websys = { version = "0.2.0", path = "transports/webtransport-websys" }
libp2p-yamux = { version = "0.45.1", path = "muxers/yamux" }
multiaddr = "0.18.1"
Expand Down
7 changes: 7 additions & 0 deletions transports/websocket-websys/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,10 @@
## 0.3.2

- Change close code in drop implementation to `1000` given that in browsers only
the code `1000` and codes between `3000` and `4999` are allowed to be set by
userland code.
See [PR 5229](https://github.com/libp2p/rust-libp2p/pull/5229).

## 0.3.1

- Add support for different WASM environments by introducing a `WebContext` that
Expand Down
2 changes: 1 addition & 1 deletion transports/websocket-websys/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ name = "libp2p-websocket-websys"
edition = "2021"
rust-version = "1.60.0"
description = "WebSocket for libp2p under WASM environment"
version = "0.3.1"
version = "0.3.2"
authors = ["Vince Vasta <vince.vasta@gmail.com>"]
license = "MIT"
repository = "https://github.com/libp2p/rust-libp2p"
Expand Down
5 changes: 3 additions & 2 deletions transports/websocket-websys/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -434,13 +434,14 @@ impl AsyncWrite for Connection {

impl Drop for Connection {
fn drop(&mut self) {
const GO_AWAY_STATUS_CODE: u16 = 1001; // See https://www.rfc-editor.org/rfc/rfc6455.html#section-7.4.1.
// In browsers, userland code is not allowed to use any other status code than 1000: https://websockets.spec.whatwg.org/#dom-websocket-close
const REGULAR_CLOSE: u16 = 1000; // See https://www.rfc-editor.org/rfc/rfc6455.html#section-7.4.1.

if let ReadyState::Connecting | ReadyState::Open = self.inner.ready_state() {
let _ = self
.inner
.socket
.close_with_code_and_reason(GO_AWAY_STATUS_CODE, "connection dropped");
.close_with_code_and_reason(REGULAR_CLOSE, "connection dropped");
}

WebContext::new()
Expand Down

0 comments on commit 0687385

Please sign in to comment.