hivemq / hivemq-mqtt-client

HiveMQ MQTT Client is an MQTT 5.0 and MQTT 3.1.1 compatible and feature-rich high-performance Java client library with different API flavours and backpressure support
https://hivemq.github.io/hivemq-mqtt-client/
Apache License 2.0
855 stars 158 forks source link

Unable to set message expiry interval with MqttPublish.NO_MESSAGE_EXPIRY due to invalid assert #580

Closed skobow closed 10 months ago

skobow commented 1 year ago

🐛 Bug Report

It is currently not possible to publish a message using MqttPublishBuilder.messageExpiryInterval(MqttPublish.NO_MESSAGE_EXPIRY) to disable message expiry instead of using MqttPublishBuilder.noMessageExpiry() as there is an invalid guard condition checking for the correct value range. As the message expiry interval is and unsigned Long value the guard condition checks for unsigned Integer instead causing the check to fail.

🔬 How To Reproduce

See code sample.

Code sample

mqtt5AsyncClient.publishWith()
            .topic(topic)
            .payload(message.getPayload())
            .messageExpiryInterval(MqttPublish.NO_MESSAGE_EXPIRY)
            .send();

Environment

irrelevant

Screenshots

📈 Expected behavior

Being able to use MqttPublish.NO_MESSAGE_EXPIRY with MqttPublishBuilder.messageExpiryInterval().

📎 Additional context

The builder itself is bypassing the guard condition when using MqttPublishBuilder.noMessageExpiry().

public @NotNull B messageExpiryInterval(final long messageExpiryInterval) {
        this.messageExpiryInterval = Checks.unsignedInt(messageExpiryInterval, "Message expiry interval");
        return self();
    }

public @NotNull B noMessageExpiry() {
    this.messageExpiryInterval = MqttPublish.NO_MESSAGE_EXPIRY;
    return self();
}
SgtSilvio commented 1 year ago

This is actually intended. The builder only allows to set values according to the MQTT specification. The message expiry interval is a four byte integer and there is no value for no expiry; instead, the message does not expire if the value is completely absent. (Link to the specification: https://docs.oasis-open.org/mqtt/mqtt/v5.0/os/mqtt-v5.0-os.html#_Toc3901064) MqttPublish.NO_MESSAGE_EXPIRY is only the value that this library uses internally to represent an absent value. Also, MqttPublish is an internal class, so should not be used at all. As this is intended behavior, I will remove the bug label. If you really need this, we can discuss making an improvement out of it.