[GitHub] [tomcat] rmaucher commented on issue #277: Refuse adding invalid HTTP 2.0 headers
rmaucher commented on issue #277: URL: https://github.com/apache/tomcat/pull/277#issuecomment-618470184 Yes, it is accurate if there's a "connection: foobar" header, then there could be a "foobar" header and in that case it's tied to the connection header. Note about my earlier "proposal" for HTTP/1.1, the connection header is used by the websocket.server.UpgradeUtil helper class to allow upgrade through the API. So it's not possible to filter it and be done, this would have to be fixed as well [not sure how]. Servlet apps can upgrade through the proper API and would not be affected. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] rmaucher commented on issue #277: Refuse adding invalid HTTP 2.0 headers
rmaucher commented on issue #277: URL: https://github.com/apache/tomcat/pull/277#issuecomment-617758397 Still, -1, again for your patch. In addition to being ugly, there's no provision in the Servlet spec to throw an exception on random header names, especially common ones, so failing, fast or slow, is wrong. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] rmaucher commented on issue #277: Refuse adding invalid HTTP 2.0 headers
rmaucher commented on issue #277: URL: https://github.com/apache/tomcat/pull/277#issuecomment-617661025 The initial post says SHOULD, but after actually checking the spec it is a MUST. https://tools.ietf.org/html/rfc7540#section-8.1.2.2 It is really odd the specification made such a choice given how many applications toy with that header in an attempt to do something useful, and given they have no idea if they're using HTTP/1.1 or HTTP/2. So this looks to me like a spec mistake [for compatibility, they should have said it has to be ignored and can be freely removed, whatever], and it is not very surprising most HTTP/2 clients would not check this requirement. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] rmaucher commented on issue #277: Refuse adding invalid HTTP 2.0 headers
rmaucher commented on issue #277: URL: https://github.com/apache/tomcat/pull/277#issuecomment-617239514 I will maintain my -1 For starters, any such HTTP/2 specific nonsense safety nets need to be added to StreamProcessor.prepareHeaders instead of other random locations. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org