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: [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]
