Copilot commented on code in PR #3099:
URL: https://github.com/apache/brpc/pull/3099#discussion_r2365558098


##########
test/brpc_protobuf_json_unittest.cpp:
##########
@@ -497,6 +499,149 @@ TEST_F(ProtobufJsonTest, json_to_pb_expected_failed_case) 
{
     ASSERT_STREQ("Invalid value `23' for optional field `Content.uid' which 
SHOULD be string, Missing required field: Ext.databyte", error.data());
 }
 
+TEST_F(ProtobufJsonTest, json_to_pb_unbounded_recursion) {
+    test::RecursiveMessage msg;
+
+    // Generate a deeply nested JSON string to trigger unbounded recursion.
+    const int recursion_depth = 140000;

Review Comment:
   The magic number 140000 should be defined as a constant with a meaningful 
name explaining why this specific depth was chosen for the test.
   ```suggestion
       // Chosen to reliably exceed the maximum recursion depth in json2pb and 
trigger error handling.
       constexpr int kUnboundedRecursionTestDepth = 140000;
       const int recursion_depth = kUnboundedRecursionTestDepth;
   ```



##########
test/brpc_protobuf_json_unittest.cpp:
##########
@@ -497,6 +499,149 @@ TEST_F(ProtobufJsonTest, json_to_pb_expected_failed_case) 
{
     ASSERT_STREQ("Invalid value `23' for optional field `Content.uid' which 
SHOULD be string, Missing required field: Ext.databyte", error.data());
 }
 
+TEST_F(ProtobufJsonTest, json_to_pb_unbounded_recursion) {
+    test::RecursiveMessage msg;
+
+    // Generate a deeply nested JSON string to trigger unbounded recursion.
+    const int recursion_depth = 140000;
+    std::string nested_json = "";
+    for (int i = 0; i < recursion_depth; ++i) {
+        nested_json += "{\"child\":";
+    }
+    nested_json += "{\"data\":\"leaf\"}";
+    for (int i = 0; i < recursion_depth; ++i) {
+        nested_json += "}";
+    }
+
+    {
+        std::string error;
+        bool ret = json2pb::JsonToProtoMessage(nested_json, &msg, &error);
+        ASSERT_FALSE(ret);
+        ASSERT_EQ("Exceeded maximum recursion depth [RecursiveMessage]", 
error);
+    }
+    {
+        json2pb::ProtoJson2PbOptions options;
+        std::string error;
+        bool ret = json2pb::ProtoJsonToProtoMessage(nested_json, &msg, 
options, &error);
+        ASSERT_FALSE(ret);
+        ASSERT_EQ("INVALID_ARGUMENT:Message too deep. Max recursion depth 
reached for key 'child'", error);
+    }
+}
+
+TEST_F(ProtobufJsonTest, pb_to_json_unbounded_recursion) {
+    test::RecursiveMessage msg;
+
+    // Create a deeply nested protobuf message.
+    const int recursion_depth = 140000;

Review Comment:
   The magic number 140000 should be defined as a constant with a meaningful 
name explaining why this specific depth was chosen for the test.



##########
test/brpc_protobuf_json_unittest.cpp:
##########
@@ -497,6 +499,149 @@ TEST_F(ProtobufJsonTest, json_to_pb_expected_failed_case) 
{
     ASSERT_STREQ("Invalid value `23' for optional field `Content.uid' which 
SHOULD be string, Missing required field: Ext.databyte", error.data());
 }
 
+TEST_F(ProtobufJsonTest, json_to_pb_unbounded_recursion) {
+    test::RecursiveMessage msg;
+
+    // Generate a deeply nested JSON string to trigger unbounded recursion.
+    const int recursion_depth = 140000;
+    std::string nested_json = "";
+    for (int i = 0; i < recursion_depth; ++i) {
+        nested_json += "{\"child\":";
+    }
+    nested_json += "{\"data\":\"leaf\"}";
+    for (int i = 0; i < recursion_depth; ++i) {
+        nested_json += "}";
+    }
+
+    {
+        std::string error;
+        bool ret = json2pb::JsonToProtoMessage(nested_json, &msg, &error);
+        ASSERT_FALSE(ret);
+        ASSERT_EQ("Exceeded maximum recursion depth [RecursiveMessage]", 
error);
+    }
+    {
+        json2pb::ProtoJson2PbOptions options;
+        std::string error;
+        bool ret = json2pb::ProtoJsonToProtoMessage(nested_json, &msg, 
options, &error);
+        ASSERT_FALSE(ret);
+        ASSERT_EQ("INVALID_ARGUMENT:Message too deep. Max recursion depth 
reached for key 'child'", error);
+    }
+}
+
+TEST_F(ProtobufJsonTest, pb_to_json_unbounded_recursion) {
+    test::RecursiveMessage msg;
+
+    // Create a deeply nested protobuf message.
+    const int recursion_depth = 140000;

Review Comment:
   The magic number 140000 should be defined as a constant with a meaningful 
name explaining why this specific depth was chosen for the test.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to