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

On Mon, Sep 25, 2023 at 09:16:34AM +1000, Martin Thomson wrote:
> (The below is forwarded with permission.)
> 
> The question below about error handling in HTTP/2 came up and I think that there is an opportunity to document things a little better.  The question is whether a server can send a response instead of treating something as a stream error.
> 
> There are two paragraphs in Section 8.1.1 that are relevant to this discussion.  I'll quote them in their entirety, because the context is important.
> 
> > Intermediaries that process HTTP requests or responses (i.e., any intermediary not acting as a tunnel) MUST NOT forward a malformed request or response. Malformed requests or responses that are detected MUST be treated as a stream error (Section 5.4.2) of type PROTOCOL_ERROR.
> >
> > For malformed requests, a server MAY send an HTTP response prior to closing or resetting the stream. Clients MUST NOT accept a malformed response.
> 
> There are four statements here:
> 
> 1. If you are an intermediary, you are responsible for ensuring that messages are well formed.  If you forward stuff without checking, that's your fault. (Not relevant to this question.)
> 
> 2. If you detect an error in a stream, you have to generate an error for that stream.
> 
> 3. A server MAY send an HTTP response before closing or resetting.  (This is the hard one and something that we might have messed up.)
> 
> 4. A malformed response is not a response. (Not relevant to this question.)
> 
> Now, take the described scenario.  A server receives a malformed request.  Does it follow (2) and generate a stream (or connection) error or can it just follow (3) and send an ordinary response?  The two clauses seem to be in tension.
> 
> If you assume that (2) applies only to intermediaries, by virtue of it being attached to a paragraph about intermediaries, then that strengthens the argument for a 4xx response being allowed.
> 
> Or, you might say that the specific conditions of (3) override previous requirements and give general license to a server to generate responses if a stream error is detected.
> 
> Either of those interpretations would seem to allow that.  The problem with that interpretation is that it doesn't fit our general philosophy in developing HTTP/2, which was to ensure that bad behaviour results in immediate feedback.  That way, bugs are driven out of the system more rapidly.  By that reasoning, a connection error is even better than a stream error.  A 4xx response in this case still provides some feedback, but the feedback is at a different layer of the stack, so it might less effective.
> 
> My understanding is that (2) is not intermediary-specific.  Also, (3) is less about giving permission, but is instead an attempt to acknowledge the possibility that a server might generate a complete response before processing an entire request stream.  That is, by the time the server detects the error, it's too late to send the error message (though it probably could generate a connection error...).  Let's say that the stream is properly formed up to a point that a server can parse out a URL, so it might know that it needs to generate a 404; or maybe the method might be unsupported on any resource on that server, resulting in a 405; or the server is overloaded and sends a 503 without processing any data from the stream; etc...  In this interpretation, the MAY is a way of saying X MAY do A or B and so Y MUST accept A or B.
> 
> Either way, this is a little clumsy.  A rearrangement of this text might make this position clearer:
> 
> > Intermediaries that process HTTP requests or responses (i.e., any intermediary not acting as a tunnel) MUST NOT forward a malformed request or response. 
> >
> > Malformed requests or responses that are detected MUST be treated as a stream error (Section 5.4.2) of type PROTOCOL_ERROR. Clients MUST NOT accept a malformed response. A malformed request might not be detected before a server sends an HTTP response. A server that has generated a response can ignore a malformed request, but MAY generate a connection error.
> 
> Ultimately, I think that Glenn's literal interpretation of the text is a defensible one, but it would be better if that interpretation were not possible.
> 
> Cheers,
> Martin
> 
> ----- Original message -----
> From: Matthew Davidson <matthew@modulolotus.net>
> To: mt@lowentropy.net, cbenfield@apple.com
> Subject: Can you clear up a question we have on RFC 9113 stream error requirements?
> Date: Friday, September 22, 2023 04:03
> 
> Hi, I'm Matthew Davidson, the maintainer of the Aleph web server. First, thank you for all the work you've put into the HTTP specs over the years.
> 
> Over in an issue for h2spec (https://github.com/summerwind/h2spec/issues/120), we're trying to determine what's required/expected/optional from a server in regards to a stream error.
> 
> The issue creator, Glenn, thinks a normal HTTP response is sufficient, and that no RST_STREAM, GOAWAY, or connection close is required, while I think an HTTP response is insufficient.
> 
> I see a lot of language in the RFC that expects a server to respond to a stream error with RST_STREAM (or GOAWAY), but our disagreement hinges on one sentence in §8.1.1:
> 
> "Malformed requests or responses that are detected MUST be treated as a stream error (Section 5.4.2) of type PROTOCOL_ERROR."
> 
> Glenn thinks this sentence applies only to intermediaries; I think it applies in general. Can you clarify?
> 
> Thanks again for all your work,
> Matthew

Martin: Thank you for expanding and posing those questions.

For others: a little bit more context:

As linked above, there is a question about what h2spec is accepting or
not accepting as valid server behavior to a malformed request.
https://github.com/summerwind/h2spec/issues/120

lighttpd src/h2.c (written prior to RFC9110) contains code and comment:

        /* RFC 7540 Section 8. HTTP Message Exchanges
         * 8.1.2.6. Malformed Requests and Responses
         *   For malformed requests, a server MAY send an HTTP
         *   response prior to closing or resetting the stream.
         * However, h2spec expects stream PROTOCOL_ERROR.
         * (This is unfortunate, since we would rather send
         *  400 Bad Request which tells client *do not* retry
         *  the bad request without modification)
         * https://github.com/summerwind/h2spec/issues/120
         * https://github.com/summerwind/h2spec/issues/121
         * https://github.com/summerwind/h2spec/issues/122
         */
      #if 0
        if (__builtin_expect( (400 == r->http_status), 0)) {
            h2_send_rst_stream(r, con, H2_E_PROTOCOL_ERROR);
            h2_retire_stream(r, con);
            /*(h2_retire_stream() invalidates r; must not use r below)*/
        }
      #endif

With the #if 0, which is lighttpd behavior in production since 2020,
lighttpd returns 400 Bad Request for a malformed request, and this
fails h2spec tests.

The above #if 0, when changed to #if 1, has lighttpd pass h2spec tests.

(The corresponding text from RFC 9113 for this topic
https://www.rfc-editor.org/rfc/rfc9113.html#name-malformed-messages
matches the RFC 7540 text from Section 8.1.2.6 quoted above.)

I coded lighttpd to send 400 Bad Request as I believe that is (slightly)
more specific than the very, very generic h2 stream PROTOCOL_ERROR, and
400 Bad Request informs the client that the request is not acceptable
as-is and should not be retried as-is.  IMO, h2 stream PROTOCOL_ERROR
does not convey that level of error detail for the malformed request.

If an HTTP response such as 400 Bad Request is sent and followed by
closing the stream, it is no longer possible to send stream error
PROTOCOL_ERROR.  The language in the RFCs currently seems to allow
for an *origin server* to send an HTTP response and to close the stream.
> > For malformed requests, a server MAY send an HTTP response prior to closing or resetting the stream.

On the other hand, *intermediaries* are required to send h2 stream
PROTOCOL_ERROR.
> > Intermediaries that process HTTP requests or responses (i.e., any intermediary not acting as a tunnel) MUST NOT forward a malformed request or response. Malformed requests or responses that are detected MUST be treated as a stream error (Section 5.4.2) of type PROTOCOL_ERROR.

(Such is my interpretation, repeated here from
 https://github.com/summerwind/h2spec/issues/120 )

Cheers, Glenn

Received on Monday, 25 September 2023 00:44:17 UTC