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


##########
src/brpc/policy/http2_rpc_protocol.cpp:
##########
@@ -649,12 +649,20 @@ H2ParseResult H2Context::OnHeaders(
             LOG(ERROR) << "Fail to insert existing stream_id=" << 
frame_head.stream_id;
             return MakeH2Error(H2_PROTOCOL_ERROR);
         } else if (rc == 2) {
-            delete sctx;
             LOG_EVERY_SECOND(WARNING)
                 << "Refused stream_id=" << frame_head.stream_id
                 << " since concurrent streams reached max_concurrent_streams="
                 << _unack_local_settings.max_concurrent_streams
                 << " on " << *_socket;
+            // The stream is refused, but its header block must still be
+            // decoded and consumed before returning the stream error. The
+            // connection stays open after REFUSED_STREAM, so skipping the
+            // block desyncs both the frame parser (the leftover header bytes
+            // would be re-read as the next frame head) and the HPACK decoder.
+            // This mirrors OnData and the client-side unknown-stream path
+            // below, which drain the payload before returning a stream error.
+            sctx->OnHeaders(it, frame_head, frag_size, pad_length);
+            delete sctx;

Review Comment:
   The return value cannot be ignored here. `H2StreamContext::OnHeaders` 
decodes through a bounded iterator and advances the outer frame iterator only 
after successful HPACK decoding; for an invalid or oversized block it returns 
an error while the payload is still unconsumed. This branch then turns that 
failure into `REFUSED_STREAM`, so the next parse iteration treats the HPACK 
bytes as a frame head and leaves the connection/HPACK state desynchronized. 
Propagate the connection-level error (or otherwise terminate after explicitly 
handling the payload) when this call fails.
   
   This issue also appears on line 664 of the same file.



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