arturobernalg commented on PR #1174: URL: https://github.com/apache/tomcat/pull/1174#issuecomment-6892464711
I don't think this should be merged as is. I tested this with a running Tomcat and a raw HTTP/2 client. The change correctly handles DATA on streams ignored after GOAWAY, but it also silently accepts invalid DATA frames. When the connection is PAUSED, unknown streams are treated as ignorable regardless of their IDs. As a result, DATA on idle streams at or below the GOAWAY Last-Stream-ID, even-numbered stream IDs and previously closed streams that have been pruned is ignored instead of causing a connection error, as it does on main. RFC 9113 section 5.1 requires a PROTOCOL_ERROR for DATA on idle streams. Section 6.8 permits discarding frames on peer-initiated streams above the GOAWAY Last-Stream-ID, but this should not bypass normal validation for other streams. Please restrict the handling to streams that can legitimately be ignored after GOAWAY and add regression tests for the invalid cases. -- 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]
