feat(web): support WebSocket subprotocols - #1970
MuNeNiCK (MuNeNiCK) wants to merge 1 commit into
Conversation
|
Automated review will not run because this contributor is not yet eligible under the automation policy. Contributors become eligible after one qualifying IronRDP pull request is merged into |
Benoît Cortier (CBenoit)
left a comment
There was a problem hiding this comment.
Hi! Thank you for the PR.
I think this is something I would like to have in the main "iron-remote-desktop" API, not as an extension. Could you change that before we merge?
Thank you!
| ] } | ||
| js-sys = "0.3" | ||
| gloo-net = { version = "0.7", default-features = false, features = ["websocket", "http", "io-util"] } | ||
| gloo-net = { version = "0.7", default-features = false, features = ["websocket", "http", "io-util", "json"] } |
There was a problem hiding this comment.
question: Why json feature is necessary now?
There was a problem hiding this comment.
The json feature was required only because gloo-net::WebSocket::open_with_protocols serializes the protocol slice through Serde. I changed the implementation to construct the browser WebSocket with web_sys::WebSocket::new_with_str_sequence and then wrap it with the existing gloo WebSocket TryFrom implementation. The json, serde, and serde_json dependencies are no longer added.
There was a problem hiding this comment.
🟢 Approval recommended
The implementation correctly preserves the default connection path, with only a non-blocking documentation-format finding.
Pull request overview
Adds configurable WebSocket subprotocol negotiation to the RDP web backend while preserving existing behavior when none are configured.
Changes:
- Exposes
webSocketProtocols()for consumers. - Validates and forwards protocol arrays when opening WebSockets.
- Documents the configuration helper.
File summaries
| File | Description |
|---|---|
web-client/iron-remote-desktop-rdp/src/main.ts |
Adds the configuration extension. |
web-client/iron-remote-desktop-rdp/README.md |
Documents subprotocol configuration. |
crates/ironrdp-web/src/session.rs |
Parses protocols and selects the appropriate WebSocket constructor. |
crates/ironrdp-web/Cargo.toml |
Enables required gloo-net JSON support. |
Cargo.lock |
Records transitive serialization dependencies. |
Review details
- Files reviewed: 4/5 changed files
- Comments generated: 1
- Review effort level: Balanced
| WebSocket endpoints use subprotocol negotiation for application protocol | ||
| selection, versioning, and handshake authentication. Pass | ||
| `webSocketProtocols()` to the configuration builder when the endpoint requires | ||
| one or more subprotocols during the opening handshake: |
There was a problem hiding this comment.
Fixed. The documentation is now in the main iron-remote-desktop README, and each sentence is kept on one source line.
d3e5730 to
2329a44
Compare
2329a44 to
5043766
Compare
|
Thank you! I moved the option into the main |
5043766 to
5b0eaef
Compare
Motivation
WebSocket subprotocol negotiation is a standard part of the opening handshake. Endpoints commonly use it for application protocol selection, version negotiation, and handshake authentication, especially in browser clients that cannot attach arbitrary request headers.
ironrdp-webcurrently creates its WebSocket internally with only the proxy URL, so consumers cannot connect to endpoints that require one or more subprotocols.Changes
webSocketProtocols()configuration extension from the RDP web packageironrdp-webWebSocket::open_with_protocolswhen protocols are configuredWebSocket::openpath when the option is omitted or emptyValidation
cargo xtask wasm check -vcargo xtask web check -vcargo xtask check fmt -vcargo xtask check lints -vcargo xtask check locks -v