Skip to content

feat(web): support WebSocket subprotocols - #1970

Open
MuNeNiCK (MuNeNiCK) wants to merge 1 commit into
Devolutions:masterfrom
MuNeNiCK:feat/websocket-subprotocols
Open

MuNeNiCK (MuNeNiCK) wants to merge 1 commit into
Devolutions:masterfrom
MuNeNiCK:feat/websocket-subprotocols

Conversation

@MuNeNiCK

Copy link
Copy Markdown
Contributor

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-web currently creates its WebSocket internally with only the proxy URL, so consumers cannot connect to endpoints that require one or more subprotocols.

Changes

  • expose a webSocketProtocols() configuration extension from the RDP web package
  • accept and validate a JavaScript string array in ironrdp-web
  • open the connection with WebSocket::open_with_protocols when protocols are configured
  • preserve the existing WebSocket::open path when the option is omitted or empty
  • document the new configuration helper

Validation

  • cargo xtask wasm check -v
  • cargo xtask web check -v
  • cargo xtask check fmt -v
  • cargo xtask check lints -v
  • cargo xtask check locks -v

@github-actions github-actions Bot added risk/low Self-contained change with no cross-crate behavioral effect scope/web Affects the web/WASM ecosystem size/S Size: up to 199 counted lines and 5 files; exceeds XS in either measure maintainer-required Maintainer review or intervention is required labels Sep 16, 2026
@github-actions

Copy link
Copy Markdown
Contributor

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 master. Maintainer review is required.

@CBenoit Benoît Cortier (CBenoit) left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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!

Comment thread crates/ironrdp-web/Cargo.toml Outdated
] }
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"] }

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

question: Why json feature is necessary now?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 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

Comment on lines +27 to +30
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:

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed. The documentation is now in the main iron-remote-desktop README, and each sentence is kept on one source line.

@MuNeNiCK

Copy link
Copy Markdown
Contributor Author

Thank you! I moved the option into the main iron-remote-desktop API. Consumers now configure it with ConfigBuilder.withWebSocketProtocols(), and the value flows through the common SessionBuilder.webSocketProtocols() API. The RDP-specific extension was removed, while an empty list still preserves the existing connection path.

@github-actions github-actions Bot added risk/medium Behavioral change that does not substantially alter a core public API size/M Size: up to 449 counted lines and 10 files; exceeds S in either measure maintainer-required Maintainer review or intervention is required and removed risk/low Self-contained change with no cross-crate behavioral effect size/S Size: up to 199 counted lines and 5 files; exceeds XS in either measure maintainer-required Maintainer review or intervention is required labels Sep 17, 2026
@MuNeNiCK
MuNeNiCK (MuNeNiCK) force-pushed the feat/websocket-subprotocols branch from 5043766 to 5b0eaef Compare September 17, 2026 01:18
@github-actions github-actions Bot added breaking-change Includes a breaking change, and requires special scrutiny at the boundaries maintainer-required Maintainer review or intervention is required and removed maintainer-required Maintainer review or intervention is required labels Sep 17, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

breaking-change Includes a breaking change, and requires special scrutiny at the boundaries maintainer-required Maintainer review or intervention is required risk/medium Behavioral change that does not substantially alter a core public API scope/web Affects the web/WASM ecosystem size/M Size: up to 449 counted lines and 10 files; exceeds S in either measure

Development

Successfully merging this pull request may close these issues.

3 participants