> 2016-09-06 17:03 GMT+03:00 Amos Jeffries <squid3 at treenet.co.nz>:
>
> > > 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.
>
> Disagree: the trunk Http::One::RequestParser::parseHttpVersionField()
> returns true, because it treats HTTP/2 message as HTTP/0.9
> message. You can see this in my Co-Advisor attachment in "HTTP/1.1
> device MUST NOT use 2.1 HTTP version in requests" test case.
>
> > > 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).
>
> Not sure that is what Http::One::RequestParser::parseHttpVersionField()
> should care of. In my understanding, this method should parse the input
> with HTTP/1 rules, returning false if the input does not conform to
> these rules.
>
> > 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.
>
> OK, and the patched Squid would return 505 (HTTP Version Not Supported)
> for HTTP/1.2+.(it is not yet implemented).
>
> > Just noticed that the unit test for "GET / HTTP/2.0" request-line is
> > also failing due to this parser change now accepting that invalid
> > version as valid.
>
> I suspect that test should be changed to check for 505 error code instead
> of 400.
>
> > You should use tok.skip(Http1::MagicPrefix) instead of trying to count
> > digits in the major version.
>
> I count digits in order to differentiate syntactically valid(but
> unsupported by Squid) HTTP versions and invalid(multi-digit)
> ones, returning 505 or 400 codes respectively.
Just a reminder of this issue. Amos, is there anything to do
to move it forward? In my point of view, only the related unit test
is to be fixed, but probably I am still misunderstand something.
Thanks,
Eduard.
_______________________________________________
squid-dev mailing list
squid-dev@lists.squid-cache.org
http://lists.squid-cache.org/listinfo/squid-dev
