From 981ea021491b4bc11dfec815c1dffeded40b5b3b Mon Sep 17 00:00:00 2001 From: Victornor <1-1@hotmail.dk> Date: Tue, 10 Dec 2024 19:37:29 +0100 Subject: [PATCH] Unused parameter in WithTryPrivate causing client to send incorrect version byte in header. (#2125) * fix: unused tryPrivate parameter in WithTryPrivate * Fix wrong usage of try private --------- Co-authored-by: christian <6939810+chkr1011@users.noreply.github.com> --- .../Formatter/V3/MqttV3PacketFormatter.cs | 37 +++++++++---------- Source/MQTTnet/Options/MqttClientOptions.cs | 2 +- .../Options/MqttClientOptionsBuilder.cs | 4 +- .../Options/MqttClientOptionsValidator.cs | 6 ++- 4 files changed, 26 insertions(+), 23 deletions(-) diff --git a/Source/MQTTnet/Formatter/V3/MqttV3PacketFormatter.cs b/Source/MQTTnet/Formatter/V3/MqttV3PacketFormatter.cs index 5e5a2f150..20dd59f49 100644 --- a/Source/MQTTnet/Formatter/V3/MqttV3PacketFormatter.cs +++ b/Source/MQTTnet/Formatter/V3/MqttV3PacketFormatter.cs @@ -18,9 +18,9 @@ public sealed class MqttV3PacketFormatter : IMqttPacketFormatter { const int FixedHeaderSize = 1; - static readonly MqttDisconnectPacket DisconnectPacket = new MqttDisconnectPacket(); + static readonly MqttDisconnectPacket DisconnectPacket = new(); - readonly MqttBufferReader _bufferReader = new MqttBufferReader(); + readonly MqttBufferReader _bufferReader = new(); readonly MqttBufferWriter _bufferWriter; readonly MqttProtocolVersion _mqttProtocolVersion; @@ -68,10 +68,8 @@ public MqttPacket Decode(ReceivedMqttPacket receivedMqttPacket) { return DecodeConnAckPacketV311(receivedMqttPacket.Body); } - else - { - return DecodeConnAckPacket(receivedMqttPacket.Body); - } + + return DecodeConnAckPacket(receivedMqttPacket.Body); case MqttControlPacketType.Disconnect: return DisconnectPacket; @@ -106,10 +104,6 @@ public MqttPacketBuffer Encode(MqttPacket packet) payload = publishPacket.Payload; remainingLength += (uint)payload.Length; } - else - { - publishPacket = null; - } var remainingLengthSize = MqttBufferWriter.GetVariableByteIntegerSize(remainingLength); @@ -481,7 +475,16 @@ byte EncodeConnectPacketV311(MqttConnectPacket packet, MqttBufferWriter bufferWr ValidateConnectPacket(packet); bufferWriter.WriteString("MQTT"); - bufferWriter.WriteByte(4); // 3.1.2.2 Protocol Level 4 + + // 3.1.2.2 Protocol Level 4 + var protocolVersion = 4; + + if (packet.TryPrivate) + { + protocolVersion |= 0x80; + } + + bufferWriter.WriteByte((byte)protocolVersion); byte connectFlags = 0x0; if (packet.CleanSession) @@ -552,19 +555,15 @@ byte EncodePacket(MqttPacket packet, MqttBufferWriter bufferWriter) { return EncodeConnectPacketV311(connectPacket, bufferWriter); } - else - { - return EncodeConnectPacket(connectPacket, bufferWriter); - } + + return EncodeConnectPacket(connectPacket, bufferWriter); case MqttConnAckPacket connAckPacket: if (_mqttProtocolVersion == MqttProtocolVersion.V311) { return EncodeConnAckPacketV311(connAckPacket, bufferWriter); } - else - { - return EncodeConnAckPacket(connAckPacket, bufferWriter); - } + + return EncodeConnAckPacket(connAckPacket, bufferWriter); case MqttDisconnectPacket _: return EncodeEmptyPacket(MqttControlPacketType.Disconnect); case MqttPingReqPacket _: diff --git a/Source/MQTTnet/Options/MqttClientOptions.cs b/Source/MQTTnet/Options/MqttClientOptions.cs index d337ea0e8..d17dd0548 100644 --- a/Source/MQTTnet/Options/MqttClientOptions.cs +++ b/Source/MQTTnet/Options/MqttClientOptions.cs @@ -127,7 +127,7 @@ public sealed class MqttClientOptions /// connect properly. /// /// - public bool TryPrivate { get; set; } = true; + public bool TryPrivate { get; set; } // Implementation in Mosquitto: https://github.com/eclipse-mosquitto/mosquitto/blob/3cbe805e71ac41a2a20cc9b2ea6b3b619f49554a/lib/send_connect.c#L153 /// /// Gets or sets the user properties. diff --git a/Source/MQTTnet/Options/MqttClientOptionsBuilder.cs b/Source/MQTTnet/Options/MqttClientOptionsBuilder.cs index a4dedff97..b1e14a797 100644 --- a/Source/MQTTnet/Options/MqttClientOptionsBuilder.cs +++ b/Source/MQTTnet/Options/MqttClientOptionsBuilder.cs @@ -361,9 +361,9 @@ public MqttClientOptionsBuilder WithTopicAliasMaximum(ushort topicAliasMaximum) /// Not all brokers support this feature so it may be necessary to set it to false if your bridge does not connect /// properly. /// - public MqttClientOptionsBuilder WithTryPrivate(bool tryPrivate = true) + public MqttClientOptionsBuilder WithTryPrivate(bool value = true) { - _options.TryPrivate = true; + _options.TryPrivate = value; return this; } diff --git a/Source/MQTTnet/Options/MqttClientOptionsValidator.cs b/Source/MQTTnet/Options/MqttClientOptionsValidator.cs index 6e9c09f9b..19b2c91c3 100644 --- a/Source/MQTTnet/Options/MqttClientOptionsValidator.cs +++ b/Source/MQTTnet/Options/MqttClientOptionsValidator.cs @@ -17,7 +17,11 @@ public static void ThrowIfNotSupported(MqttClientOptions options) if (options.ProtocolVersion == MqttProtocolVersion.V500) { - // Everything is supported. + if (options.TryPrivate) + { + throw new NotSupportedException("Feature TryPrivate only works with MQTT version 3.1 and 3.1.1."); + } + return; }