From f54bf7aeb06c6a4cf9350482f7814c3fb9c4fd82 Mon Sep 17 00:00:00 2001 From: Duke Bartholomew Date: Thu, 21 Jan 2021 14:40:45 +0100 Subject: [PATCH] Allow to use int while build MqttMessageIdVariableHeader (#10930) Motivation: Exception occurs in the MQTT message builder class (`io.netty.handler.codec.mqtt.MqttMessageBuilders`) when trying to create a message with packetId > 32767 Modification: -add method that takes int - deprecate old methods that take short. Result: Fixes #10929 . --- .../codec/mqtt/MqttMessageBuilders.java | 36 +++++- .../mqtt/MqttMessageBuildersPacketIdTest.java | 115 ++++++++++++++++++ 2 files changed, 145 insertions(+), 6 deletions(-) create mode 100644 codec-mqtt/src/test/java/io/netty/handler/codec/mqtt/MqttMessageBuildersPacketIdTest.java diff --git a/codec-mqtt/src/main/java/io/netty/handler/codec/mqtt/MqttMessageBuilders.java b/codec-mqtt/src/main/java/io/netty/handler/codec/mqtt/MqttMessageBuilders.java index 8c854f0bf0..a2eea43774 100644 --- a/codec-mqtt/src/main/java/io/netty/handler/codec/mqtt/MqttMessageBuilders.java +++ b/codec-mqtt/src/main/java/io/netty/handler/codec/mqtt/MqttMessageBuilders.java @@ -524,7 +524,7 @@ public final class MqttMessageBuilders { public static final class PubAckBuilder { - private short packetId; + private int packetId; private byte reasonCode; private MqttProperties properties; @@ -536,11 +536,19 @@ public final class MqttMessageBuilders { return this; } - public PubAckBuilder packetId(short packetId) { + public PubAckBuilder packetId(int packetId) { this.packetId = packetId; return this; } + /** + * @deprecated use {@link PubAckBuilder#packetId(int)} instead + */ + @Deprecated + public PubAckBuilder packetId(short packetId) { + return packetId(packetId & 0xFFFF); + } + public PubAckBuilder properties(MqttProperties properties) { this.properties = properties; return this; @@ -557,18 +565,26 @@ public final class MqttMessageBuilders { public static final class SubAckBuilder { - private short packetId; + private int packetId; private MqttProperties properties; private final List grantedQoses = new ArrayList(); SubAckBuilder() { } - public SubAckBuilder packetId(short packetId) { + public SubAckBuilder packetId(int packetId) { this.packetId = packetId; return this; } + /** + * @deprecated use {@link SubAckBuilder#packetId(int)} instead + */ + @Deprecated + public SubAckBuilder packetId(short packetId) { + return packetId(packetId & 0xFFFF); + } + public SubAckBuilder properties(MqttProperties properties) { this.properties = properties; return this; @@ -604,18 +620,26 @@ public final class MqttMessageBuilders { public static final class UnsubAckBuilder { - private short packetId; + private int packetId; private MqttProperties properties; private final List reasonCodes = new ArrayList(); UnsubAckBuilder() { } - public UnsubAckBuilder packetId(short packetId) { + public UnsubAckBuilder packetId(int packetId) { this.packetId = packetId; return this; } + /** + * @deprecated use {@link UnsubAckBuilder#packetId(int)} instead + */ + @Deprecated + public UnsubAckBuilder packetId(short packetId) { + return packetId(packetId & 0xFFFF); + } + public UnsubAckBuilder properties(MqttProperties properties) { this.properties = properties; return this; diff --git a/codec-mqtt/src/test/java/io/netty/handler/codec/mqtt/MqttMessageBuildersPacketIdTest.java b/codec-mqtt/src/test/java/io/netty/handler/codec/mqtt/MqttMessageBuildersPacketIdTest.java new file mode 100644 index 0000000000..c63e771f00 --- /dev/null +++ b/codec-mqtt/src/test/java/io/netty/handler/codec/mqtt/MqttMessageBuildersPacketIdTest.java @@ -0,0 +1,115 @@ +/* + * Copyright 2020 The Netty Project + * + * The Netty Project licenses this file to you under the Apache License, + * version 2.0 (the "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at: + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations + * under the License. + */ + +package io.netty.handler.codec.mqtt; + +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; + +import java.util.Arrays; + +import static org.junit.Assert.*; + +@RunWith(value = Parameterized.class) +public class MqttMessageBuildersPacketIdTest { + @Parameterized.Parameter + public Integer id; + + @Parameterized.Parameters(name = "{index}: {0}") + public static Iterable data() { + // we take a subset of valid packetIds + return Arrays.asList( + 0x0001, + 0x000F, + 0x00FF, + 0x0FFF, + 0xFFFF + ); + } + + @Test + public void testUnsubAckMessageIdAsShort() { + final MqttUnsubAckMessage msg = MqttMessageBuilders.unsubAck() + .packetId(id.shortValue()) + .build(); + + assertEquals( + id.intValue(), + msg.variableHeader().messageId() + ); + } + + @Test + public void testSubAckMessageIdAsShort() { + final MqttSubAckMessage msg = MqttMessageBuilders.subAck() + .packetId(id.shortValue()) + .build(); + + assertEquals( + id.intValue(), + msg.variableHeader().messageId() + ); + } + + @Test + public void testPubAckMessageIdAsShort() { + final MqttMessage msg = MqttMessageBuilders.pubAck() + .packetId(id.shortValue()) + .build(); + + assertEquals( + id.intValue(), + ((MqttMessageIdVariableHeader) msg.variableHeader()).messageId() + ); + } + + @Test + public void testUnsubAckMessageIdAsInt() { + final MqttUnsubAckMessage msg = MqttMessageBuilders.unsubAck() + .packetId(id) + .build(); + + assertEquals( + id.intValue(), + msg.variableHeader().messageId() + ); + } + + @Test + public void testSubAckMessageIdAsInt() { + final MqttSubAckMessage msg = MqttMessageBuilders.subAck() + .packetId(id) + .build(); + + assertEquals( + id.intValue(), + msg.variableHeader().messageId() + ); + } + + @Test + public void testPubAckMessageIdAsInt() { + final MqttMessage msg = MqttMessageBuilders.pubAck() + .packetId(id) + .build(); + + assertEquals( + id.intValue(), + ((MqttMessageIdVariableHeader) msg.variableHeader()).messageId() + ); + } +}