Some more compliant connect error handling

Wiebe Cazemier
1 parent 4eaca771
Showing 6 changed files with 46 additions and 2 deletions
client.cpp
client.h
mainapp.cpp
mqttpacket.cpp
utils.cpp
utils.h
@@ -100,6 +100,9 @@ void Client::writeMqttPacket(const MqttPacket &amp;packet)
     assert(wwi >= static_cast<int>(packet.getSizeIncludingNonPresentHeader()));
     assert(wwi <= static_cast<int>(writeBufsize));
+    if (packet.packetType == PacketType::DISCONNECT)
+        setReadyForDisconnect();
+
     setReadyForWriting(true);
 }
@@ -36,6 +36,7 @@ class Client
     bool connectPacketSeen = false;
     bool readyForWriting = false;
     bool readyForReading = true;
+    bool disconnectWhenBytesWritten = false;
     std::string clientid;
     std::string username;
@@ -123,6 +124,10 @@ public:
     void writePingResp();
     void writeMqttPacket(const MqttPacket &packet);
     bool writeBufIntoFd();
+    bool readyForDisconnecting() const { return disconnectWhenBytesWritten && wwi == wri && wwi == 0; }
+
+    // Do this before calling an action that makes this client ready for writing, so that the EPOLLOUT will handle it.
+    void setReadyForDisconnect() { disconnectWhenBytesWritten = true; }
     std::string repr();
@@ -74,6 +74,9 @@ void do_thread_work(ThreadData *threadData)
                         {
                             if (!client->writeBufIntoFd())
                                 threadData->removeClient(client);
+
+                            if (client->readyForDisconnecting())
+                                threadData->removeClient(client);
                         }
                     }
                     catch(std::exception &ex)
@@ -128,7 +128,12 @@ void MqttPacket::handleConnect()
         }
         else
         {
-            throw ProtocolError("Only MQTT 3.1 and 3.1.1 supported.");
+            ConnAck connAck(ConnAckReturnCodes::UnacceptableProtocolVersion);
+            MqttPacket response(connAck);
+            sender->setReadyForDisconnect();
+            sender->writeMqttPacket(response);
+            std::cout << "Rejecting because of invalid protocol version: " << sender->repr() << std::endl;
+            return;
         }
         char flagByte = readByte();
@@ -174,7 +179,28 @@ void MqttPacket::handleConnect()
             password = std::string(readBytes(password_length), password_length);
         }
-        // TODO: validate UTF8 encoded username/password.
+        // The specs don't really say what to do when client id not UTF8, so including here.
+        if (!isValidUtf8(client_id) || !isValidUtf8(username) || !isValidUtf8(password))
+        {
+            ConnAck connAck(ConnAckReturnCodes::MalformedUsernameOrPassword);
+            MqttPacket response(connAck);
+            sender->setReadyForDisconnect();
+            sender->writeMqttPacket(response);
+            std::cout << "Client ID, username or passwords has invalid UTF8: " << sender->repr() << std::endl;
+            return;
+        }
+
+        // In case the client_id ever appears in topics.
+        // TODO: make setting?
+        if (strContains(client_id, "+") || strContains(client_id, "#"))
+        {
+            ConnAck connAck(ConnAckReturnCodes::ClientIdRejected);
+            MqttPacket response(connAck);
+            sender->setReadyForDisconnect();
+            sender->writeMqttPacket(response);
+            std::cout << "ClientID has + or # in the id: " << sender->repr() << std::endl;
+            return;
+        }
         sender->setClientProperties(client_id, username, true, keep_alive);
         sender->setWill(will_topic, will_payload, will_retain, will_qos);
@@ -106,3 +106,8 @@ bool isValidUtf8(const std::string &amp;s)
     }
     return multibyte_remain == 0;
 }
+
+bool strContains(const std::string &s, const std::string &needle)
+{
+    return s.find(needle) != std::string::npos;
+}
@@ -25,4 +25,6 @@ bool topicsMatch(const std::string &amp;subscribeTopic, const std::string &amp;publishTo
 bool isValidUtf8(const std::string &s);
+bool strContains(const std::string &s, const std::string &needle);
+
 #endif // UTILS_H