Convert ConnectData.username to std::optional

Intead of having the flag boolean. This made it logically safer to later change the username, in X509 handling.
halfgaar · Nov 4, 2023 · 1f14b1d · 1f14b1d
1 parent b94ffb9
commit 1f14b1d
Show file tree

Hide file tree

Showing 2 changed files with 19 additions and 16 deletions.
diff --git a/mqttpacket.cpp b/mqttpacket.cpp
@@ -512,7 +512,7 @@ ConnectData MqttPacket::parseConnectData()
     if (reserved)
         throw ProtocolError("Protocol demands reserved flag in CONNECT is 0", ReasonCodes::MalformedPacket);
 
-    result.user_name_flag = !!(flagByte & 0b10000000);
+    bool user_name_flag = static_cast<bool>(flagByte & 0b10000000);
     result.password_flag = !!(flagByte & 0b01000000);
     result.will_retain = !!(flagByte & 0b00100000);
     result.will_qos = (flagByte & 0b00011000) >> 3;
@@ -666,22 +666,22 @@ ConnectData MqttPacket::parseConnectData()
         result.willpublish.payload = std::string(readBytes(will_payload_length), will_payload_length);
     }
 
-    if (result.user_name_flag)
+    if (user_name_flag)
     {
         result.username = readBytesToString(false);
-        result.willpublish.username = result.username;
+        result.willpublish.username = result.username.value();
 
-        if (result.username.empty())
+        if (result.username.value().empty())
             throw ProtocolError("Username flagged as present, but it's 0 bytes.", ReasonCodes::MalformedPacket);
 
-        if (!settings.allowUnsafeUsernameChars && containsDangerousCharacters(result.username))
-            throw ProtocolError(formatString("Username '%s' contains unsafe characters and 'allow_unsafe_username_chars' is false.", result.username.c_str()),
+        if (!settings.allowUnsafeUsernameChars && containsDangerousCharacters(result.username.value()))
+            throw ProtocolError(formatString("Username '%s' contains unsafe characters and 'allow_unsafe_username_chars' is false.", result.username.value().c_str()),
                                 ReasonCodes::BadUserNameOrPassword);
     }
 
     if (result.password_flag)
     {
-        if (this->protocolVersion <= ProtocolVersion::Mqtt311 && !result.user_name_flag)
+        if (this->protocolVersion <= ProtocolVersion::Mqtt311 && !result.username)
         {
             throw ProtocolError("MQTT 3.1.1: If the User Name Flag is set to 0, the Password Flag MUST be set to 0.");
         }
@@ -802,15 +802,16 @@ void MqttPacket::handleConnect()
 
     ConnectData connectData = parseConnectData();
 
+    std::string username = connectData.username ? connectData.username.value() : "";
+
     if (sender->getX509ClientVerification() > X509ClientVerification::None)
     {
         std::optional<std::string> certificateUsername = sender->getUsernameFromPeerCertificate();
 
         if (!certificateUsername || certificateUsername.value().empty())
             throw ProtocolError("Client certificate did not provider username", ReasonCodes::BadUserNameOrPassword);
 
-        connectData.user_name_flag = true;
-        connectData.username = certificateUsername.value();
+        username = certificateUsername.value();
     }
 
     sender->setBridge(connectData.bridge);
@@ -836,13 +837,13 @@ void MqttPacket::handleConnect()
 
     // I deferred the initial UTF8 check on username to be able to give an appropriate connack here, but to me, the specs
     // are actually vague whether 'BadUserNameOrPassword' should be given on invalid UTF8.
-    if (!isValidUtf8(connectData.username))
+    if (connectData.username && !isValidUtf8(connectData.username.value()))
     {
         ConnAck connAck(protocolVersion, ReasonCodes::BadUserNameOrPassword);
         MqttPacket response(connAck);
         sender->setReadyForDisconnect();
         sender->writeMqttPacket(response);
-        logger->logf(LOG_ERR, "Username has invalid UTF8: %s", connectData.username.c_str());
+        logger->logf(LOG_ERR, "Username has invalid UTF8: %s", connectData.username.value().c_str());
         return;
     }
 
@@ -882,7 +883,7 @@ void MqttPacket::handleConnect()
         clientIdGenerated = true;
     }
 
-    sender->setClientProperties(protocolVersion, connectData.client_id, connectData.username, true, connectData.keep_alive,
+    sender->setClientProperties(protocolVersion, connectData.client_id, username, true, connectData.keep_alive,
                                 connectData.max_outgoing_packet_size, connectData.max_outgoing_topic_aliases);
 
     if (settings.willsEnabled && connectData.will_flag)
@@ -933,7 +934,7 @@ void MqttPacket::handleConnect()
     AuthResult authResult = AuthResult::login_denied;
     std::string authReturnData;
 
-    if (!connectData.user_name_flag && connectData.authenticationMethod.empty() && settings.allowAnonymous)
+    if (!connectData.username && connectData.authenticationMethod.empty() && settings.allowAnonymous)
     {
         authResult = AuthResult::success;
     }
@@ -944,7 +945,7 @@ void MqttPacket::handleConnect()
     }
     else if (connectData.authenticationMethod.empty())
     {
-        authResult = authentication.unPwdCheck(connectData.client_id, connectData.username, connectData.password, getUserProperties(), sender);
+        authResult = authentication.unPwdCheck(connectData.client_id, username, connectData.password, getUserProperties(), sender);
     }
     else
     {

diff --git a/packetdatatypes.h b/packetdatatypes.h
@@ -11,15 +11,17 @@ See LICENSE for license details.
 #ifndef PACKETDATATYPES_H
 #define PACKETDATATYPES_H
 
+#include <optional>
+
 #include "mqtt5properties.h"
 
+
 struct ConnectData
 {
     uint8_t protocol_level_byte = 0;
     bool bridge = false;
 
     // Flags
-    bool user_name_flag = false;
     bool password_flag = false;
     bool will_retain = false;
     uint8_t will_qos = false;
@@ -41,7 +43,7 @@ struct ConnectData
     // Content from Payload
     std::string client_id;
     WillPublish willpublish;
-    std::string username;
+    std::optional<std::string> username;
     std::string password;
 
     Mqtt5PropertyBuilder builder;