-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Subscription Identifier always included when adding an mqtt topic on MQTT 5 #9214
Comments
I'm not fully understand the report. |
Hi @artembilan and thank you for the quick response, what I mean is that the subscription identifier must not be used in the subscription message from the client if the broker does not support subscription identifiers. Just to make sure, this issues addresses only the usage of mqtt 5 connections. According to the specification of mqtt5, section '3.2.2.3.12 Subscription Identifiers Available' (https://docs.oasis-open.org/mqtt/mqtt/v5.0/os/mqtt-v5.0-os.html#_Toc3901092) the broker can support the usage of subscription identifiers but it's not a must. In addition there is a configuration option in the MqttConnectionOptions of the mqtt5 paho client, that allows to configure the usage of subscription identifiers (https://github.com/eclipse/paho.mqtt.java/blob/master/org.eclipse.paho.mqttv5.client/src/main/java/org/eclipse/paho/mqttv5/client/MqttConnectionOptions.java#L64 // https://github.com/eclipse/paho.mqtt.java/blob/master/org.eclipse.paho.mqttv5.client/src/main/java/org/eclipse/paho/mqttv5/client/MqttConnectionOptions.java#L894). In my scenario I connected to an mqtt 5 broker which does not support the usage of subscription identifiers. I also configured the MqttConnectionOptions to not use subscription identifiers. Yet the subscription message contains the parameter of the subscription identifier, when I used the method for adding a topic. Best regards, |
OK. I see the logic:
I wonder then why have we introduced that Looks like it is a default behavior anyway if Testing... |
OK. So, we just used wrong API to subscribe.
And if you choose Will provide the fix shortly. |
…nChannelAdapter` Fixes: #9214 The `MqttAsyncClient` can set `subscriptionIdentifier` from session if it is enabled and available from connection * Remove manual `subscriptionIdentifierCounter` from the `Mqttv5PahoMessageDrivenChannelAdapter`. Instead, use `subscriptionProperties.setSubscriptionIdentifiers(List.of(0));` to make the `this.mqttSession.getNextSubscriptionIdentifier();` to work when `if (connOpts.useSubscriptionIdentifiers() && this.mqttConnection.isSubscriptionIdentifiersAvailable()) {` is `true` (cherry picked from commit 3dcbdef)
…nChannelAdapter` Fixes: #9214 The `MqttAsyncClient` can set `subscriptionIdentifier` from session if it is enabled and available from connection * Remove manual `subscriptionIdentifierCounter` from the `Mqttv5PahoMessageDrivenChannelAdapter`. Instead, use `subscriptionProperties.setSubscriptionIdentifiers(List.of(0));` to make the `this.mqttSession.getNextSubscriptionIdentifier();` to work when `if (connOpts.useSubscriptionIdentifiers() && this.mqttConnection.isSubscriptionIdentifiersAvailable()) {` is `true` (cherry picked from commit 3dcbdef) # Conflicts: # spring-integration-mqtt/src/main/java/org/springframework/integration/mqtt/inbound/Mqttv5PahoMessageDrivenChannelAdapter.java
@artembilan @wilkinsona @spring-builds @MarcelFadtke I am using 6.3.0 version of spring-mqtt-integration, but still facing the same issue. Unable to receive message on shared topics. |
…ahoMessageDrivenChannelAdapter` Fixes: spring-projects#9214 The `MqttAsyncClient` can set `subscriptionIdentifier` from session if it is enabled and available from connection * Remove manual `subscriptionIdentifierCounter` from the `Mqttv5PahoMessageDrivenChannelAdapter`. Instead, use `subscriptionProperties.setSubscriptionIdentifiers(List.of(0));` to make the `this.mqttSession.getNextSubscriptionIdentifier();` to work when `if (connOpts.useSubscriptionIdentifiers() && this.mqttConnection.isSubscriptionIdentifiersAvailable()) {` is `true` **Auto-cherry-pick to `6.3.x` & `6.2.x`**
In what version(s) of Spring Integration are you seeing this issue?
6.3.0.RELEASE
(likely since / caused by #8879)
Describe the bug
When adding a topic to the subscriptions, the following section causes the inclusion of a subscription identifier even if the server sent that the subscription identifiers are not available (during the connection by the Connect Ack response) or if the paho mqtt client connection options are configured to not use subscription identifiers.
Code:
spring-integration/spring-integration-mqtt/src/main/java/org/springframework/integration/mqtt/inbound/Mqttv5PahoMessageDrivenChannelAdapter.java
Lines 346 to 350 in 91de12c
To Reproduce
Setup an mqtt 5 client and connect to a broker and adding a topic programatically, while tracking the mqtt message properties with wireshark.
Expected behavior
The method includes mqtt connection settings from the server's Connect Ack message and the configured client MqttConnectionOption, so that the subscription identifier usage can be disabled / turned off.
Sample
A link to a GitHub repository with a minimal, reproducible sample.
Reports that include a sample will take priority over reports that do not.
At times, we may require a sample, so it is good to try and include a sample up front.
The text was updated successfully, but these errors were encountered: