On 6/09/2016 9:06 a.m., Eduard Bagdasaryan wrote: > Hello, > > This patch fixes Squid to return 505 (Version Not Supported) error > code for HTTP/2+ versions. > > Before this change, when a syntactically valid HTTP/1 request indicated > HTTP major version 2, Squid mangled and forwarded the request as an > HTTP/1 message.
That is not true. The trunk code returns false the same as this patched version. Because the version field for HTTP/2 does not start with the static magic string "HTTP/1." > Since Squid does not and cannot correctly interpret an > HTTP/1 request using HTTP/2 rules, returning an error and closing the > connection appears to be the only correct action possible. That is not correct either. The old logic requires method to be GET for full HTTP/0.9 handling. HTTP/2 pseudo-method is PRI. If anything other than PRI is seen it is not valid HTTP/2. At most we should have logic passing HTTP/2 transparently through the proxy (configurable). Which is the relay behaviour of HTTP/0.9 processing, but not its header mangling behaviour (because PRI, not GET method). In the case where it is not HTTP/1.1, HTTP/1.0, HTTP/2, or GET without a version field. Then the version must start with "HTTP/" to be considered invalid HTTP version and the MUST clause applicable. Otherwise it could be any protocol at all. HTTP/2, SMTP, Finger, Secure-HTTP, WebSocket, SPDY and ICAP can all be sent to this parse logic at various times. Also, HTTP/2 does not obsolete HTTP/1.x RFC's, so there is an explicit possibility that HTTP/1.2+ will exist at some point. The IETF WG expectation is/was that 1.2+ (if any) would contain compatibility for HTTP/2+ features over HTTP/1.x syntax. > > Also: since HTTPbis prohibits HTTP versions with multi-digits, treat > such requests as invalid. > > Attached Co-Advisor result pages for patched and unpatched Squid. > For some reason I'm not sure of yet these changes result in unit test failure parsing a garbage line with various bad whitespace characters in it. make check with testHttp1Parser debugging enabled produces: " TEST @1071, in="\r \t \n" testHttp1Parser.cc:106:Assertion Test name: testHttp1Parser::testParseRequestLineProtocols equality assertion failed - Expected: 0 - Actual : 1 " Amos _______________________________________________ squid-dev mailing list [email protected] http://lists.squid-cache.org/listinfo/squid-dev
