From 66c83f7b74b3ea28baa4a0702d621aba8331993a Mon Sep 17 00:00:00 2001 From: Subhobrata Dey Date: Mon, 5 Jun 2017 17:32:44 -0700 Subject: [PATCH] codec.mqtt: password and willMessage field types should be byte[] Motivation: Update the mqtt-codec based on mqtt spec (3.1.3.5). Modification: Changes made to the file MqttConnectPayload.java. Subsequent changes have been made to files MqttDecoder.java, MqttEncoder.java, MqttMessageBuilders.java. Test cases have been updated. Result: Fixes #6750 . --- .../codec/mqtt/MqttConnectPayload.java | 39 ++++++++++++++++++- .../netty/handler/codec/mqtt/MqttDecoder.java | 27 ++++++------- .../netty/handler/codec/mqtt/MqttEncoder.java | 8 ++-- .../codec/mqtt/MqttMessageBuilders.java | 23 ++++++++++- .../handler/codec/mqtt/MqttCodecTest.java | 7 ++++ 5 files changed, 81 insertions(+), 23 deletions(-) diff --git a/codec-mqtt/src/main/java/io/netty/handler/codec/mqtt/MqttConnectPayload.java b/codec-mqtt/src/main/java/io/netty/handler/codec/mqtt/MqttConnectPayload.java index 1e16acf443..e274e3f29e 100644 --- a/codec-mqtt/src/main/java/io/netty/handler/codec/mqtt/MqttConnectPayload.java +++ b/codec-mqtt/src/main/java/io/netty/handler/codec/mqtt/MqttConnectPayload.java @@ -16,6 +16,7 @@ package io.netty.handler.codec.mqtt; +import io.netty.util.CharsetUtil; import io.netty.util.internal.StringUtil; /** @@ -25,16 +26,34 @@ public final class MqttConnectPayload { private final String clientIdentifier; private final String willTopic; - private final String willMessage; + private final byte[] willMessage; private final String userName; - private final String password; + private final byte[] password; + /** + * @deprecated use {@link MqttConnectPayload#MqttConnectPayload(String, String, byte[], String, byte[])} instead + */ + @Deprecated public MqttConnectPayload( String clientIdentifier, String willTopic, String willMessage, String userName, String password) { + this( + clientIdentifier, + willTopic, + willMessage.getBytes(CharsetUtil.UTF_8), + userName, + password.getBytes(CharsetUtil.UTF_8)); + } + + public MqttConnectPayload( + String clientIdentifier, + String willTopic, + byte[] willMessage, + String userName, + byte[] password) { this.clientIdentifier = clientIdentifier; this.willTopic = willTopic; this.willMessage = willMessage; @@ -50,7 +69,15 @@ public final class MqttConnectPayload { return willTopic; } + /** + * @deprecated use {@link MqttConnectPayload#willMessageInBytes()} instead + */ + @Deprecated public String willMessage() { + return new String(willMessage, CharsetUtil.UTF_8); + } + + public byte[] willMessageInBytes() { return willMessage; } @@ -58,7 +85,15 @@ public final class MqttConnectPayload { return userName; } + /** + * @deprecated use {@link MqttConnectPayload#passwordInBytes()} instead + */ + @Deprecated public String password() { + return new String(password, CharsetUtil.UTF_8); + } + + public byte[] passwordInBytes() { return password; } diff --git a/codec-mqtt/src/main/java/io/netty/handler/codec/mqtt/MqttDecoder.java b/codec-mqtt/src/main/java/io/netty/handler/codec/mqtt/MqttDecoder.java index b734c349a0..c8a3546e0e 100644 --- a/codec-mqtt/src/main/java/io/netty/handler/codec/mqtt/MqttDecoder.java +++ b/codec-mqtt/src/main/java/io/netty/handler/codec/mqtt/MqttDecoder.java @@ -339,21 +339,21 @@ public final class MqttDecoder extends ReplayingDecoder { int numberOfBytesConsumed = decodedClientId.numberOfBytesConsumed; Result decodedWillTopic = null; - Result decodedWillMessage = null; + Result decodedWillMessage = null; if (mqttConnectVariableHeader.isWillFlag()) { decodedWillTopic = decodeString(buffer, 0, 32767); numberOfBytesConsumed += decodedWillTopic.numberOfBytesConsumed; - decodedWillMessage = decodeAsciiString(buffer); + decodedWillMessage = decodeByteArray(buffer); numberOfBytesConsumed += decodedWillMessage.numberOfBytesConsumed; } Result decodedUserName = null; - Result decodedPassword = null; + Result decodedPassword = null; if (mqttConnectVariableHeader.hasUserName()) { decodedUserName = decodeString(buffer); numberOfBytesConsumed += decodedUserName.numberOfBytesConsumed; } if (mqttConnectVariableHeader.hasPassword()) { - decodedPassword = decodeString(buffer); + decodedPassword = decodeByteArray(buffer); numberOfBytesConsumed += decodedPassword.numberOfBytesConsumed; } @@ -419,17 +419,6 @@ public final class MqttDecoder extends ReplayingDecoder { return decodeString(buffer, 0, Integer.MAX_VALUE); } - private static Result decodeAsciiString(ByteBuf buffer) { - Result result = decodeString(buffer, 0, Integer.MAX_VALUE); - final String s = result.value; - for (int i = 0; i < s.length(); i++) { - if (s.charAt(i) > 127) { - return new Result(null, result.numberOfBytesConsumed); - } - } - return new Result(s, result.numberOfBytesConsumed); - } - private static Result decodeString(ByteBuf buffer, int minBytes, int maxBytes) { final Result decodedSize = decodeMsbLsb(buffer); int size = decodedSize.value; @@ -445,6 +434,14 @@ public final class MqttDecoder extends ReplayingDecoder { return new Result(s, numberOfBytesConsumed); } + private static Result decodeByteArray(ByteBuf buffer) { + final Result decodedSize = decodeMsbLsb(buffer); + int size = decodedSize.value; + byte[] bytes = new byte[size]; + buffer.readBytes(bytes); + return new Result(bytes, decodedSize.numberOfBytesConsumed + size); + } + private static Result decodeMsbLsb(ByteBuf buffer) { return decodeMsbLsb(buffer, 0, 65535); } diff --git a/codec-mqtt/src/main/java/io/netty/handler/codec/mqtt/MqttEncoder.java b/codec-mqtt/src/main/java/io/netty/handler/codec/mqtt/MqttEncoder.java index 4ee7293075..967cdb844b 100644 --- a/codec-mqtt/src/main/java/io/netty/handler/codec/mqtt/MqttEncoder.java +++ b/codec-mqtt/src/main/java/io/netty/handler/codec/mqtt/MqttEncoder.java @@ -113,8 +113,8 @@ public final class MqttEncoder extends MessageToMessageEncoder { // Will topic and message String willTopic = payload.willTopic(); byte[] willTopicBytes = willTopic != null ? encodeStringUtf8(willTopic) : EmptyArrays.EMPTY_BYTES; - String willMessage = payload.willMessage(); - byte[] willMessageBytes = willMessage != null ? encodeStringUtf8(willMessage) : EmptyArrays.EMPTY_BYTES; + byte[] willMessage = payload.willMessageInBytes(); + byte[] willMessageBytes = willMessage != null ? willMessage : EmptyArrays.EMPTY_BYTES; if (variableHeader.isWillFlag()) { payloadBufferSize += 2 + willTopicBytes.length; payloadBufferSize += 2 + willMessageBytes.length; @@ -126,8 +126,8 @@ public final class MqttEncoder extends MessageToMessageEncoder { payloadBufferSize += 2 + userNameBytes.length; } - String password = payload.password(); - byte[] passwordBytes = password != null ? encodeStringUtf8(password) : EmptyArrays.EMPTY_BYTES; + byte[] password = payload.passwordInBytes(); + byte[] passwordBytes = password != null ? password : EmptyArrays.EMPTY_BYTES; if (variableHeader.hasPassword()) { payloadBufferSize += 2 + passwordBytes.length; } 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 51be0072b5..b893205ee7 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 @@ -17,6 +17,7 @@ package io.netty.handler.codec.mqtt; import io.netty.buffer.ByteBuf; import io.netty.buffer.Unpooled; +import io.netty.util.CharsetUtil; import java.util.ArrayList; import java.util.List; @@ -77,9 +78,9 @@ public final class MqttMessageBuilders { private boolean willRetain; private MqttQoS willQos = MqttQoS.AT_MOST_ONCE; private String willTopic; - private String willMessage; + private byte[] willMessage; private String username; - private String password; + private byte[] password; ConnectBuilder() { } @@ -119,7 +120,16 @@ public final class MqttMessageBuilders { return this; } + /** + * @deprecated use {@link ConnectBuilder#willMessage(byte[])} instead + */ + @Deprecated public ConnectBuilder willMessage(String willMessage) { + willMessage(willMessage.getBytes(CharsetUtil.UTF_8)); + return this; + } + + public ConnectBuilder willMessage(byte[] willMessage) { this.willMessage = willMessage; return this; } @@ -145,7 +155,16 @@ public final class MqttMessageBuilders { return this; } + /** + * @deprecated use {@link ConnectBuilder#password(byte[])} instead + */ + @Deprecated public ConnectBuilder password(String password) { + password(password.getBytes(CharsetUtil.UTF_8)); + return this; + } + + public ConnectBuilder password(byte[] password) { this.hasPassword = true; this.password = password; return this; diff --git a/codec-mqtt/src/test/java/io/netty/handler/codec/mqtt/MqttCodecTest.java b/codec-mqtt/src/test/java/io/netty/handler/codec/mqtt/MqttCodecTest.java index 8bfd2fb95d..21a69055ed 100644 --- a/codec-mqtt/src/test/java/io/netty/handler/codec/mqtt/MqttCodecTest.java +++ b/codec-mqtt/src/test/java/io/netty/handler/codec/mqtt/MqttCodecTest.java @@ -28,6 +28,7 @@ import org.junit.Test; import org.mockito.Mock; import org.mockito.MockitoAnnotations; +import java.util.Arrays; import java.util.LinkedList; import java.util.List; @@ -395,7 +396,13 @@ public class MqttCodecTest { actual.clientIdentifier()); assertEquals("MqttConnectPayload UserName mismatch ", expected.userName(), actual.userName()); assertEquals("MqttConnectPayload Password mismatch ", expected.password(), actual.password()); + assertTrue( + "MqttConnectPayload Password bytes mismatch ", + Arrays.equals(expected.passwordInBytes(), actual.passwordInBytes())); assertEquals("MqttConnectPayload WillMessage mismatch ", expected.willMessage(), actual.willMessage()); + assertTrue( + "MqttConnectPayload WillMessage bytes mismatch ", + Arrays.equals(expected.willMessageInBytes(), actual.willMessageInBytes())); assertEquals("MqttConnectPayload WillTopic mismatch ", expected.willTopic(), actual.willTopic()); }