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

Fixed message id generation according to spec #138

Merged
merged 1 commit into from
Nov 21, 2013

Conversation

mantoni
Copy link

@mantoni mantoni commented Oct 28, 2013

Changed id generation from random to increasing by one - as suggested by
the specification. Using random numbers does not guarantee that the same
id isn't used twice. Also made sure that 0 is never used since it's an
invalid message id.

For details see section 2.4 of the spec:
http://public.dhe.ibm.com/software/dw/webservices/ws-mqtt/mqtt-v3r1.html#msg-id

Changed id generation from random to increasing by one - as suggested by
the specification. Using random numbers does not guarantee that the same
id isn't used twice. Also made sure that 0 is never used since it's an
invalid message id.

For details see section 2.4 of the spec:
http://public.dhe.ibm.com/software/dw/webservices/ws-mqtt/mqtt-v3r1.html#msg-id
@mcollina
Copy link
Member

Have you run into real-world problems for this?

Just one little note: having a fixed starting id is very bad from a security point of view. So, make that random and we can merge :).

@mantoni
Copy link
Author

mantoni commented Oct 29, 2013

No, not that I've run into any real issues. Just thought it's not a great idea if two consecutive messages have the same ID.

Will change to random initial ID tonight. Thanks.

@mcollina mcollina merged commit 504f1fc into mqttjs:master Nov 21, 2013
@mcollina
Copy link
Member

Fixed and merged in! Thanks!

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

Successfully merging this pull request may close these issues.

2 participants