MB-68697: Sanitise agent name

The agent name is pushed into a JSON object as part of the
connections stat request.

HELO must ensure that we don't store an agent name which
cannot be represent in JSON.

Change-Id: Ibbb375ba84b8af9b1edbf4792dd40894efae3432
Reviewed-on: https://review.couchbase.org/c/kv_engine/+/234361
Reviewed-by: Trond Norbye <[email protected]>
Well-Formed: Restriction Checker
Tested-by: Jim Walker <[email protected]>
Reviewed-by: Mohammad Zaeem <[email protected]>

Author: jimwwalker

daemon/protocol/mcbp/hello_packet_executor.cc

@@ -223,11 +223,18 @@ void process_hello_packet_executor(Cookie& cookie) {
                 if (obj != json.end() && obj->is_string()) {
                     connection.setAgentName(obj->get<std::string>());
                 }
-            } catch (const nlohmann::json::exception&) {
-                connection.setAgentName(key);
+            } catch (const nlohmann::json::exception& e) {
+                // MB-68697: Not valid JSON, sanitise the key as this could be
+                // invalid UTF-8 which could later crash stat requests.
+                LOG_INFO("{}: Failed to parse agent name: {}",
+                         connection.getId(),
+                         e.what());
+                connection.setAgentName(req.getPrintableKey());
             }
         } else {
-            connection.setAgentName(key);
+            // MB-68697: Store a sanitised version of the key as the agent name
+            // may be invalid UTF-8 which could later crash stat requests.
+            connection.setAgentName(req.getPrintableKey());
         }
 
         log_buffer.append("[");

protocol/mcbp/request.cc

@@ -61,7 +61,7 @@ std::string Request::getPrintableKey() const {
 
     std::string buffer{reinterpret_cast<const char*>(key.data()), key.size()};
     for (auto& ii : buffer) {
-        if (!std::isgraph(ii)) {
+        if (!std::isprint(ii)) {
             ii = '.';
         }
     }

tests/testapp/testapp_stats.cc

@@ -555,3 +555,35 @@ TEST_P(StatsTest, MB59260) {
     EXPECT_EQ(max_system, stats["max_system_connections"]);
     EXPECT_EQ(max_user, stats["max_user_connections"]);
 }
+
+TEST_P(StatsTest, MB68697_json) {
+    // Do a RAW hello with a junked JSON string
+    BinprotHelloCommand cmd("{\"a\":\"8\xf8\"}");
+    const auto resp = BinprotHelloResponse(adminConnection->execute(cmd));
+    ASSERT_TRUE(resp.isSuccess());
+
+    auto stats = adminConnection->stats("connections");
+    // We have at _least_ 2 connections
+    ASSERT_LE(2, stats.size());
+
+    stats = adminConnection->stats("connections self");
+    ASSERT_EQ(1, stats.size());
+    // See sanitised agent name
+    EXPECT_EQ(R"({"a":"8."})", stats.front()["agent_name"].get<std::string>());
+}
+
+TEST_P(StatsTest, MB68697_raw) {
+    // Do a RAW hello with a junked string that later cannot be turned into JSON
+    BinprotHelloCommand cmd("\"a\":\"8\xf8\"");
+    const auto resp = BinprotHelloResponse(adminConnection->execute(cmd));
+    ASSERT_TRUE(resp.isSuccess());
+
+    auto stats = adminConnection->stats("connections");
+    // We have at _least_ 2 connections
+    ASSERT_LE(2, stats.size());
+
+    stats = adminConnection->stats("connections self");
+    ASSERT_EQ(1, stats.size());
+    // See sanitised agent name
+    EXPECT_EQ(R"("a":"8.")", stats.front()["agent_name"].get<std::string>());
+}