Re: Can servers generate responses to malformed requests in h2?

On Mon, Sep 25, 2023 at 03:58:03PM +1000, Martin Thomson wrote:
> On Mon, Sep 25, 2023, at 13:33, Glenn Strauss wrote:
> > My understanding is that I could treat all HTTP request header errors
> > received as HTTP/2 requests as h2 stream error PROTOCOL_ERROR.  If that
> > is what is desired by the RFC authors, please issue an update or errata
> > and I'll change that code in lighttpd.
> 
> That was never really the intent.  The goal of HTTP/2 was to limit stream errors to those cases where errors are found in HTTP/2-specific stuff (frame format and ordering mainly).

Makes sense.

> This encroaches into "headers" in only two ways: pseudo-fields and Content-Length.

These are data inside the HEADERS/CONTINUATION frame and require parsing
of the data.  Parsing the (HPACK-decoded) data from inside the frame is
a different layer than h2 framing, even if it is specific to the h2 spec

> > However, I currently am unable to understand why h2 PROTOCOL_ERROR
> > (or H3_GENERAL_PROTOCOL_ERROR) is somehow better than 400 Bad Request.
> 
> I think that your "another reason" below might point to why this is preferable.  A client that makes a request via a proxy that upgrades it to HTTP/2 or HTTP/3 is not responsible for errors that arise in translation to the target request format, so it is in no position to correct that error.  The error is something that the proxy caused and so the proxy needs to handle it.

In my example, I was trying to convey the case where an HTTP/1.0 client
sent a request that was rejected by the strict origin server, and the
proxy was not able to detect that the request would be rejected (since
the request could be rejected by some strict security policy, not only
for malformed request).

You correctly summarized my intent in a prior message:
>> How clients consume such errors is a very good point, and what seems
>> to motivate Glenn's position also.

> Maybe that's no real help to a client who is dealing with a busted proxy and who is presented with a request that times out or whatever the proxy does in that case.  But it's about ensuring that the error is appropriately targeted.  If the proxy isn't implementing the protocol properly, then the developers of that proxy really do need to pay attention.

There are many proxies and quality varies.  Not everything is
CDN carrier-grade and compliant with the specs.  Third-party corporate
proxies can be deficient with regards to the specs and also very
difficult and time consuming to get the vendor to fix, release, and
then have corporate clients upgrade.

> Protocol errors like this should not occur during normal operation.

PROTOCOL_ERROR can be sent by a server for many different reasons.

https://www.rfc-editor.org/rfc/rfc9113#name-error-handling
"Additionally, an endpoint MAY use any applicable error code when it detects an error condition; a generic error code (such as PROTOCOL_ERROR or INTERNAL_ERROR) can always be used in place of more specific error codes."

HTTP 400 Bad Request is more specific, IMHO, and applicable when parsing
higher up in the HTTP application stack, above the h2 framing layer, and
above the HPACK-decoding layer.

> Yes, PROTOCOL_ERROR isn't exactly the nicest error (which is partly why closing the connection might be preferable, because you get to include a reason phrase), but that only suggests that we might want to explore ways of making errors clearer or more actionable.

Sure.

> > I can see where h2 specification around pseudo-headers is specific to
> > the h2 protocol.  However, an implementation "could" HPACK-decode the
> > entire HEADERS frame into a single string that looks almost identical
> > to HTTP/1.1 request headers with the exception of the addition of
> > pseudo-headers.  It could then be loosely parsed as HTTP/1.x request
> > headers.  
> 
> If you are rewriting HTTP/2 into HTTP/1.x, you need all the pseudo-fields in order to make sense of the message.  If one is missing, then your converter can generate an error (which you can send as a stream error).  Same if a pseudo-field arrives late.
> 
> > An implementation might need to fully parse the HEADERS frame
> > before being able to determine that a required pseudo-header is missing.
> > If it has already gotten this far parsing the HTTP request, why should
> > the RFC disallow the implementation from returning an HTTP error code?
> 
> The specification requires that pseudo-fields precede real fields, so this is not really an option.

I am trying to convey that this is part of the HTTP request parsing,
which to me is a much higher layer in the stack from h2 framing.
In implementations, there is a lot of potential code overlap with
HTTP/1.x parsing and that code often produces HTTP error codes.

If a client sends a request over HTTP/2 using a ":method" not recognized
by lighttpd, then HTTP error code 501 is returned and parsing of the
request stops at that point.  Duplicate psuedo-headers would not be
detected since an HTTP error response has already been set.

If a duplicate ":method" pseudo-header is sent (with a valid :method),
then rejecting that is handled in the same layer near that code or
later on.  The HTTP request is being parsed at this point.  This is well
above the layer of h2 framing and HPACK-decoding.  I would argue that a
connection error would be an extremely imprecise response.  A duplicate
":method" is a bad request and should be rejected.  I would have to
needlessly rewrite or duplicate code to treat ":method" duplicated
pseudo-header as an h2 PROTOCOL_ERROR instead of as 400 Bad Request like
I do for a duplicated "content-length".  Perhaps related: are you
suggesting that for HTTP/2, I should treat duplicate "content-length" as
h2 PROTOCOL_ERROR, and for HTTP/1.x, I should continue to treat
duplicated "content-length" as 400 Bad Request?  That would sound like
an unnecessary difference without distinction.


HTTP error codes are understood by HTTP/1.0 and well-handled by proxies.
I think a server should be able to allowed to send an HTTP response at
this layer, rather than having to save this state for a lower layer to
have to check for and reject the bad request with a (potentially less
useful) generic h2 PROTOCOL_ERROR.

Cheers, Glenn

Received on Monday, 25 September 2023 07:32:51 UTC