You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@brpc.apache.org by "ubeddulla (via GitHub)" <gi...@apache.org> on 2026/07/19 08:12:00 UTC

[PR] read GOAWAY fields before Additional Debug Data in OnGoAway (brpc)

ubeddulla opened a new pull request, #3395:
URL: https://github.com/apache/brpc/pull/3395

   ### What problem does this PR solve?
   
   Issue Number: N/A
   
   Problem Summary:
   
   1. RFC 7540 section 6.8 puts Last-Stream-ID and Error Code at the front of the GOAWAY payload with the Additional Debug Data after them, but `OnGoAway` skips `payload_size - 8` bytes first and then reads the trailing 8 bytes, so a peer that attaches debug data gets to choose the Last-Stream-ID we act on.
   2. It also keeps the reserved high bit, so a `0xff` debug byte makes `last_stream_id` negative, and `_goaway_stream_id` then never passes the `>= 0` test in `TryToInsertStream`, leaving the client free to open new streams on a connection the peer has already abandoned.
   
   ### What is changed and the side effects?
   
   Changed: moved the two reads ahead of the forward and masked the reserved bit, the way `ConsumeFrameHead` and `OnWindowUpdate` already treat theirs. The new `http2_goaway_with_debug_data` case sends a GOAWAY announcing stream 1 with 8 bytes of debug data and checks `_goaway_stream_id` ends up as 1 instead of -1.
   
   Side effects:
   - Performance effects: none, same number of bytes consumed from the iterator.
   
   - Breaking backward compatibility: no. A GOAWAY carrying no debug data parses exactly as before.
   


-- 
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: dev-unsubscribe@brpc.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For additional commands, e-mail: dev-help@brpc.apache.org

Re: [PR] read GOAWAY fields before Additional Debug Data in OnGoAway (brpc)

Posted by "wwbmmm (via GitHub)" <gi...@apache.org>.
wwbmmm commented on PR #3395:
URL: https://github.com/apache/brpc/pull/3395#issuecomment-5029367987

   LGTM


-- 
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: dev-unsubscribe@brpc.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For additional commands, e-mail: dev-help@brpc.apache.org

Re: [PR] read GOAWAY fields before Additional Debug Data in OnGoAway (brpc)

Posted by "chenBright (via GitHub)" <gi...@apache.org>.
chenBright merged PR #3395:
URL: https://github.com/apache/brpc/pull/3395


-- 
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: dev-unsubscribe@brpc.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For additional commands, e-mail: dev-help@brpc.apache.org