eclipse / paho.mqtt.python

paho.mqtt.python
Other
2.17k stars 723 forks source link

Maybe improper implemention of "Host" in websocket header #666

Closed hulb closed 1 year ago

hulb commented 2 years ago

https://github.com/eclipse/paho.mqtt.python/blob/9782ab81fe7ee3a05e74c7f3e1d03d5611ea4be4/src/paho/mqtt/client.py#L3726

at here, the Host of header will be conbined with host and port. let's say if I want to connect to wss://mqtt.example.com, and I should write the code like below

mqttc.tls_set()
mqttc.connect("mqtt.example.com", port=443)

then actual request header Host would be

Host: mqtt.example.com:443

It's quite unusual for a tls request brings the port in Host header if the port is 443. And the golang implemention Eclipse Paho Go doesn't do the same.

And it brings me a problem. I use nginx at the front of a real mqtt server, And when I try to connect it through wss,it can't handle the Host (with the port of 443) properly.

ralight commented 2 years ago

Are you sure this is the cause of the problem? A Host header with a port in it is standard, even if it is the default value. I've just tested with Paho Python connecting to a WSS -> nginx -> mosquitto and it all worked fine for me. Perhaps there is some aspect of your configuration that is different.

hulb commented 2 years ago

Yes,I compare the request when using the emqx-client and paho.mqtt.python, the Host in header is the difference, and emqx-client can connect to the server as expected. And when I added a extra header to overrite the default Host in header when create the client instance, things worked well. Since I don't control the server side which I tried to connect, I guess the server maybe did something with the Host.

A Host header with a port in it is standard

Yes.Maybe the wrong side is the server that I wanted to connect. I just noticed that other client(implemented in golang) seems using a difference way to make the request header.

oliverrahner commented 2 years ago

I just stumbled across the same issue, but with a different perspective. I am connecting to an AWS IoT MQTT broker. Authentication includes a signature that spans the host header, so it has to match the expected value to the bit. They also don't seem to expect seeing :443 as part of the host header.

My workaround:

headers = {"Host": host}
mqtt_client.ws_set_options(path=path, headers=headers)
mqtt_client.connect(host=host, port=port)
ralight commented 1 year ago

Thank you, this is now fixed and will be part of the next release.

PierreF commented 7 months ago

I don't think it's valid to always remove the port.

If no port is included, the default port for the service requested is implied (e.g., 443 for an HTTPS URL, and 80 for an HTTP URL). -- https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/host

The RFC said:

URI producers and normalizers should omit the port component [...] if its value would be the same as that of the scheme's default -- https://www.rfc-editor.org/rfc/rfc3986#section-3.2.3

The RFC isn't very clear whether this apply to Host header. Anyway if we connect to wss://hostname:1234 I don't see why we should not include port in the Host header. All client I known (browsers, curl, Python requests) don't omit it, they only omit it when it's the default value. I'll change code so that port is only omitted when it's value is 80 for http and 443 for https.