[rumqttc] MqttOptions::parse_url broken for websocket urls

[brocaar]

I believe these two options are equal:

let options = MqttOptions::new("123", "localhost", 1883);
let options = MqttOptions::parse_url("mqtt://example.com:1883?client_id=123").unwrap();

The url crate is used to parse the transport, hostname and port. Then the client_id is retrieved by reading the URL parameters, etc...

let host = url.host_str().unwrap_or_default().to_owned();

https://github.com/bytebeamio/rumqtt/blob/main/rumqttc/src/v5/mod.rs#L581

However, when looking at the websocket example, the host argument of MqttOptions::new(...) is not the hostname, but the full websocket URL:

    // port parameter is ignored when scheme is websocket
    let mut mqttoptions = MqttOptions::new(
        "clientId-aSziq39Bp3",
        "ws://broker.mqttdashboard.com:8000/mqtt",
        8000,
    );
    mqttoptions.set_transport(Transport::Ws);

https://github.com/bytebeamio/rumqtt/blob/main/rumqttc/examples/websocket.rs#L10

If using the parse_url for ws://broker.mqttdashboard.com:8000/mqtt?client_id=clientId-aSziq39Bp3 then this would result in host being set to broker.mqttdashboard.com, not ws://broker.mqttdashboard.com:8000/mqtt.

The result is the following error being returned by the eventloop:

• Invalid Url: Couldn't parse host from url.

[swanandx]

Hey, thanks for reporting, the way we handle ws / wss differently from tcp is the root of this.

I think it is handled that way because when using websockets, address can have path like /mqtt, which isn't case with normal tcp. So when we treat host as full URL, this can be done:

rumqtt/rumqttc/src/eventloop.rs

Line 403 in b8766e1

let mut request = options.broker_addr.as_str().into_client_request()?;

to work around it, instead of using broker_addr directly to convert to request, we can separately extract the path ( if present ) and do:

let mut request = format!("ws://{domain}:{port}/{path}").into_client_request()?;

^ this would allow us to not ignore the port when using websocket and to address this issue.

PS:

I have opened a PR to refactor MqttOptions: #789

this PR would allow users to provide partial / full urls for both tcp & ws! it is still draft because I'm waiting for feedback on design, like shall we return error if we fail to parse, or shall we panic. ( more details in PR desc )

please have a look at that PR as well!

Thank you!