-
Notifications
You must be signed in to change notification settings - Fork 110
Added topic alias support. #660
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
Conversation
7bc0517
to
a0f6334
Compare
Codecov Report
@@ Coverage Diff @@
## master #660 +/- ##
==========================================
+ Coverage 82.13% 82.68% +0.55%
==========================================
Files 45 46 +1
Lines 7012 7076 +64
==========================================
+ Hits 5759 5851 +92
+ Misses 1253 1225 -28 |
d9b5c4c
to
be46eb9
Compare
I'm not sure that this is actually a safe change to make. What happens if an MQTT client attempts to bind the topic alias a second time? Could there be MQTT clients that attempt to use a topic alias upon first connecting to test that the broker does not have the topic alias? |
1392cee
to
365fd76
Compare
The exising topic - topic_alias mapping is overritten. The spec said that as follows:
See https://docs.oasis-open.org/mqtt/mqtt/v5.0/os/mqtt-v5.0-os.html#_Toc3901113
No, it is a protocol violation.
See https://docs.oasis-open.org/mqtt/mqtt/v5.0/os/mqtt-v5.0-os.html#_Toc3901120 |
|
TopicAlias lifetime is the same as Session lifetime It is different from MQTT v5 spec but practical choice. See https://lists.oasis-open.org/archives/mqtt-comment/202009/msg00000.html Related fix: When topic is empty and no topic alias entry is found, then protocol_error. The broker send DISCONNECT packet with protocol_error reason code. process_disconnect implementation was simply doesn't call `on_mqtt_message_processed()` but it was bad. It caused exit from ioc.run() loop because the next async_read was not called. The correct behavior is calling `on_mqtt_message_processed()` and close socket. The endpoint can detect the socket close by on_error(). Fixed topic alias.
365fd76
to
ffdc21e
Compare
I will merge the PR tomorrow morning (JST). |
TopicAlias lifetime is the same as Session lifetime
It is different from MQTT v5 spec but practical choice.
See
https://lists.oasis-open.org/archives/mqtt-comment/202009/msg00000.html
Related fix:
When topic is empty and no topic alias entry is found, then
protocol_error.
The broker send DISCONNECT packet with protocol_error reason code.
process_disconnect implementation was simply doesn't call
on_mqtt_message_processed()
but it was bad.It caused exit from ioc.run() loop because the next async_read was not
called.
The correct behavior is calling
on_mqtt_message_processed()
and closesocket. The endpoint can detect the socket close by on_error().