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


##########
src/brpc/redis_reply.cpp:
##########
@@ -141,6 +141,12 @@ ParseError RedisReply::ConsumePartialIOBuf(butil::IOBuf& 
buf, int depth) {
             return PARSE_ERROR_NOT_ENOUGH_DATA;
         }
         const size_t len = str.size() - 1;
+        if (FLAGS_redis_max_allocation_size < 0 ||
+            len > (size_t)FLAGS_redis_max_allocation_size) {
+            LOG(ERROR) << "simple string exceeds max allocation size! max="
+                       << FLAGS_redis_max_allocation_size << ", actually=" << 
len;
+            return PARSE_ERROR_ABSOLUTELY_WRONG;

Review Comment:
   The new redis_max_allocation_size cap only runs after CRLF is found. If the 
peer streams a very large simple/error string without CRLF (or with CRLF split 
across reads), this branch will keep returning PARSE_ERROR_NOT_ENOUGH_DATA and 
allow buf to grow well past the configured limit, unlike RedisCommandParser 
which enforces the limit while waiting for CRLF. Consider enforcing the 
allocation cap in the cut_until-not-found path too (with a one-byte '\r' 
allowance to avoid rejecting a boundary CRLF split).



-- 
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