Unused parameter in WithTryPrivate causing client to send incorrect v…

…ersion byte in header. (#2125) * fix: unused tryPrivate parameter in WithTryPrivate * Fix wrong usage of try private --------- Co-authored-by: christian <[email protected]>
dotnet · Dec 10, 2024 · 981ea02 · 981ea02
1 parent 8566fd5
commit 981ea02
Show file tree

Hide file tree

Showing 4 changed files with 26 additions and 23 deletions.
diff --git 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
@@ -127,7 +127,7 @@ public sealed class MqttClientOptions
     ///         connect properly.
     ///     </remarks>
     /// </summary>
-    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
 
     /// <summary>
     ///     Gets or sets the user properties.

diff --git 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.
     /// </summary>
-    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
@@ -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;
         }