Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Error in zero length client id when using 3.1.1 #78

Closed
karlw00t opened this issue Jul 4, 2017 · 2 comments
Closed

Error in zero length client id when using 3.1.1 #78

karlw00t opened this issue Jul 4, 2017 · 2 comments

Comments

@karlw00t
Copy link

karlw00t commented Jul 4, 2017

We ran into a issue over at home-assistant/core#8315 where users were getting errors after upgrading the mqtt client. I did a bit of digging on the subject and I think there is a bug in hbmqtt's implementation of version 3.1.1 of the MQTT protocol. It appears that 3.1.1 allows for clients to send a zero length client id to the server, and according to the spec:

A Server MAY allow a Client to supply a ClientId that has a length of zero bytes, however if it does so the Server MUST treat this as a special case and assign a unique ClientId to that Client. It MUST then process the CONNECT packet as if the Client had provided that unique ClientId

It appears when getting the handler in the broker [1] it checks if client_id is True, but it's a zero length string, so the function returns None.

I think there are two options here, refuse zero length client ids, or generate a random_id and store that for later use.

@njouanin
Copy link
Owner

njouanin commented Jul 4, 2017

I think we should generate an unique ID and proceed with the connection. Could you try to fix this and provide a PR ?

@karlw00t
Copy link
Author

Are there design doc anywhere? I'm trying to wrap my head around what's going on, but there a few places ([1] [2]) where variable names are not clear. My current question is what does "parent" mean on an instance of a Session class? It appears that if we can't find a cached session [3], then we should do the same client_id gen that's done here [4].

[1] https://github.com/karlw00t/hbmqtt/blob/master/hbmqtt/session.py#L84
[2] https://github.com/karlw00t/hbmqtt/blob/master/hbmqtt/broker.py#L371
[3] https://github.com/karlw00t/hbmqtt/blob/master/hbmqtt/broker.py#L374
[4] https://github.com/karlw00t/hbmqtt/blob/master/hbmqtt/broker.py#L367

@mi3z mi3z mentioned this issue Oct 23, 2017
njouanin pushed a commit that referenced this issue Nov 10, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants