send WebSocketConfig headers, subprotocols and user agent in the upgrade request (#165)

[bryan-ashby]

Fixes #165. (I filed the issue as @NuSkooler; this is my work account.)

WebSocketTransport::connect built the upgrade request from a fixed set of headers, so with_header, with_subprotocol(s) and with_user_agent never reached the server. This PR sends them, and adds tests that check the request a server actually receives.

Changes

• Request construction moves to WebSocketConfig::build_handshake_request (public, so callers can check a configuration without connecting). It starts from tungstenite's into_client_request, then adds:
  • Sec-WebSocket-Protocol: the configured subprotocols in order, followed by mqtt when the list doesn't include it (MQTT-6.0.0-3);
  • User-Agent, when set. The default is now mqtt5/<crate version>, replacing the stale mqtt-v5/0.4.0, since it now actually goes on the wire;
  • every custom header.
• Host now includes a non-default port. The old code used host_str(), so ws://broker:8080 sent Host: broker. Starting from into_client_request also strips userinfo and keeps IPv6 brackets.
• Validation happens when the request is built, before dialing. with_header keeps its infallible signature. Errors are MqttError::Configuration and name the header. A request is rejected when:
  • a header name or value is invalid. This also blocks CR/LF injection through a value;
  • a custom header names one the transport sets itself (Host, Connection, Upgrade, Sec-WebSocket-Version, Sec-WebSocket-Key). tungstenite would already refuse a duplicate of these, but only at connect time, with a less clear error;
  • a custom header is Sec-WebSocket-Extensions (the transport negotiates no extensions and could not speak one a server accepted) or Sec-WebSocket-Accept (it belongs in the server's response);
  • a custom header is Content-Length, Transfer-Encoding or Trailer. The upgrade request has no body, and a server told otherwise, e.g. Transfer-Encoding: chunked, would wait for one;
  • a custom header is a hop-by-hop or expectation header: TE, Keep-Alive, Proxy-Connection or Expect;
  • Sec-WebSocket-Protocol or User-Agent is set as a custom header. The error points to the dedicated setter, so each value has one source;
  • two custom header names differ only in case. headers is a HashMap<String, String>, so which copy won would otherwise depend on iteration order;
  • a subprotocol is not an HTTP token, or is listed more than once (RFC 6455 §4.1). For example "a, b" would otherwise silently become two subprotocols.
• Custom headers are validated and sent in name order, so the request and the error a bad configuration reports don't depend on HashMap iteration order.
• WebSocketConfig has a hand-written Debug that lists header names but not values, since values often carry credentials.
• Doc fixes. subprotocol()'s doc comment said "negotiated", but it returns the first configured subprotocol. I've corrected the comment and left the behaviour alone (see below). The subprotocol setters now cite MQTT-6.0.0-3.

Behavior changes for existing users

Because the configuration now reaches the wire, some existing setups will behave differently. The CHANGELOG has a "may affect existing configurations" entry covering both:

• Bad headers. An invalid or reserved custom header that used to be silently dropped now makes connect fail.
• User-Agent. Every WebSocket connection, including MqttClient's, now sends a User-Agent header.

Subprotocols and MQTT-6.0.0-3

mqtt is always offered: it's appended after the configured subprotocols when the list doesn't include it. The server still selects one value, so a broker that wants mqttv5.0 can still choose it from mqttv5.0, mqtt, while one that only selects mqtt, like this crate's, still connects. A subprotocol listed twice is rejected.

Tests

• Unit tests on build_handshake_request: defaults, applied configuration, the empty-list fallback, unset user agent, Host (default port, userinfo, IPv6), mqtt appended after other subprotocols, duplicate subprotocols rejected, every rejection case, deterministic header order (checked across freshly seeded maps; it fails without the sort), and Debug redaction.
• Property test prop_websocket_config_with_options now also checks the built request. It expects an error exactly when the generated headers include a restricted or case-duplicated name, or a subprotocol repeats, and otherwise expects the configured subprotocols followed by mqtt when absent. A new prop_handshake_request_rejects_restricted_headers covers those names in both cases.
• tests/websocket_handshake_integration.rs connects the transport to a loopback accept_hdr_async server and checks the upgrade request the server received. All three tests fail against main:
  • configured headers: Authorization arrives as None;
  • defaults: User-Agent arrives as None. This test also checks Host against the exact address the server bound;
  • reserved header: connect dials and is refused instead of rejecting the config.
• preferring_another_subprotocol_still_connects_to_the_broker connects with_subprotocol("mqttv5.0") to this crate's MqttBroker WebSocket listener (bound to port 0). Without the mqtt fallback it fails with Server sent no subprotocol.

Locally, fmt, clippy (workspace and wasm), cargo doc -D warnings and the full test suite pass on the pinned 1.98.0 toolchain, apart from tests that fail on this machine only, identically on main: another service holds 127.0.0.1:20000, the first port TestBroker hands out, and a local broker holds 127.0.0.1:1883, which the external-broker tests expect to be empty or theirs. For a clean run, upstream's Rust CI and Conformance Suite workflows run on the fork and pass: bryan-ashby#1.

Not in this PR

• MqttClient support. ClientInner::connect_websocket{,_secure} builds WebSocketConfig::new(url) from the URL alone, so headers set this way only help callers that drive WebSocketTransport directly. ConnectOptions::with_websocket_config, as agreed on WebSocket transport ignores WebSocketConfig headers, subprotocols, and user_agent #165, is in add ConnectOptions::with_websocket_config for MqttClient (#165) #210; the request modifier will follow separately.
• The URL password in Debug and logs. A ws://user:pass@host URL still shows its password in WebSocketConfig's Debug and in connect's tracing span, exactly as before; only header values are newly redacted.
• Reporting the subprotocol the server selected. subprotocol() could return the server's choice once connected. That changes what two existing tests rely on, so I've left it out; I can add it here or separately.
• WSS TLS settings being ignored. That's WSS transport ignores WebSocketConfig::tls_config (custom CA, client certificate, ALPN) #190, fixed in use WebSocketConfig's TLS configuration for wss:// connections (#190) #191.

The CHANGELOG entry stays under [Unreleased], as you asked.

🤖 Generated with Claude Code

[fabracht]

Thanks, this looks good. One change I'd like before merging:

With with_subprotocol("mqttv5.0") the request now carries only mqttv5.0, and the broker in this repo answers without a subprotocol, so the client fails with SubProtocol error: Server sent no subprotocol. On main the same config connects because mqtt was always sent. MQTT-6.0.0-3 also says the client must offer mqtt. Can you always include it, appended if the caller didn't list it? The server still picks one value, so a broker that wants mqttv5.0 can still select it. The CHANGELOG note about adding mqtt to the list can go then.

Two minor ones:

• with_subprotocols(&["mqtt", "mqtt"]) sends mqtt, mqtt. RFC 6455 §4.1 requires unique values.
• TE, Keep-Alive, Proxy-Connection, Trailer and Expect get through as custom headers. They should go in the same list as Connection and Transfer-Encoding.

Returning http::Request from build_handshake_request is fine, the request modifier will need http types anyway. Leave the [Unreleased] heading as is, I'll move it when I release 0.46.0.

We found these with throwaway tests, so please double check them on your side before changing anything:

• Subprotocol: start an MqttBroker with a websocket listener (BrokerConfig::with_websocket, as in tests/broker_websocket_integration.rs) and connect a WebSocketTransport with WebSocketConfig::new(url)?.with_subprotocol("mqttv5.0"). It connects on main and fails on this branch. with_subprotocols(&["mqttv5.0", "mqtt"]) connects on both.
• The other two only need build_handshake_request(): with_subprotocols(&["mqtt", "mqtt"]) gives Sec-WebSocket-Protocol: mqtt, mqtt, and with_header("TE", "trailers") builds without an error.

[bryan-ashby]

Thanks for the review. I reproduced all three points against main and this branch before changing anything, and they matched what you saw. Fixed in 0f6b48f:

• mqtt is always offered. It's appended after the configured subprotocols when the list doesn't include it, so with_subprotocol("mqttv5.0") sends mqttv5.0, mqtt. The CHANGELOG note about adding mqtt yourself is gone. A new test, preferring_another_subprotocol_still_connects_to_the_broker, connects that config to an MqttBroker WebSocket listener; it fails with Server sent no subprotocol without the change.
• Duplicate subprotocols are rejected with a Configuration error, the same way duplicate custom headers are.
• TE, Trailer, Keep-Alive, Proxy-Connection and Expect are reserved, alongside Connection and Transfer-Encoding.

I merged main (49ea04a) to resolve the CHANGELOG conflict; the entry stays under [Unreleased]. The fork's Rust CI and Conformance runs are on bryan-ashby#1.

Per your answer on #165, ConnectOptions::with_websocket_config is up as #210. The request modifier will follow separately.

[fabracht]

Thanks, Bryan, all three are fixed. I checked them here too: with_subprotocol("mqttv5.0") now connects to our broker and fails again if I take out the mqtt append, with_subprotocols(&["mqtt", "mqtt"]) is rejected, and TE, Trailer, Keep-Alive, Proxy-Connection and Expect all fail as custom headers.

Main moved this week (0.47.1 to 0.47.3), so the CHANGELOG conflicted again. I merged main into your branch and kept your entry under [Unreleased], so there's nothing left for you to do here. Merging now.