Improve Expect: 100-continue header handling

[yoshuawuyts]

tldr: We're currently always accepting Expect: 100-continue headers, where we should be enabling header validation before we send back an intermediate 100 status code.

Expected behavior

The 100 continue header exists in order to signal to a client that the request has been understood, and all the headers are correct. This enables validating things like encoding, offsets, and authentication before proceeding to transfer a significant amount of data.

The validation of these headers should be defined by end-users, as they are the ones that will know which combination of headers is acceptable.

Current behavior

async-h1 is currently hardcoded to always reply with 100 to the Expect: 100-continue header. This means end-users don't have a chance to validate headers before proceeding.

async-h1/src/server/decode.rs

Lines 132 to 144 in 3a368bd

	const EXPECT_HEADER_VALUE: &str = "100-continue";
	const EXPECT_RESPONSE: &[u8] = b"HTTP/1.1 100 Continue\r\n\r\n";
	async fn handle_100_continue<IO>(req: &Request, io: &mut IO) -> http_types::Result<()>
	where
	IO: Write + Unpin,
	{
	if let Some(EXPECT_HEADER_VALUE) = req.header(EXPECT).map(|h| h.as_str()) {
	io.write_all(EXPECT_RESPONSE).await?;
	}
	Ok(())
	}

Implementation

We should provide some way for end-users to validate headers before sending back the 100 continue response. I'm not quite sure how to do this, but one option would be to handle only send over 100-continue when the first chunk of the request body is requested. That would indicate that the framework has successfully parsed the headers, and is now ready to receive the body.

If a Response is returned before the Request body has finished, that would likely carry a non-100 status code, and the client would know not to send the request body. So semantically I think that would be the right way to go?

If at all possible I think we should evade exposing the Expect: 100-continue semantics to end-users, since the back and forth dance is quite complex, needs to work out of the box, and doesn't fit well with the req/res model we use in http-rs.

[jbr]

This makes sense. Do you think that this is a situation where applications might want to change the default behavior? Some apps might want to check that the request is valid before receiving the body, and some might prefer not to take the small performance hit and always immediately begin receiving the body

I'm going to make a try at this because it sounds interesting, but it seems very likely to conflict with @Yarn's #134, which isn't ready to merge and touches a lot of files

[jbr]

Tiny bit of spec-lawyering: I'm not convinced that the word "correct" in the title is accurate. I think the type of validation proposed here is optional origin server behavior. Here's the relevant part of rfc2616§8:

      - Upon receiving a request which includes an Expect request-header
        field with the "100-continue" expectation, an origin server MUST
        either respond with 100 (Continue) status and continue to read
        from the input stream, or respond with a final status code. The
        origin server MUST NOT wait for the request body before sending
        the 100 (Continue) response. If it responds with a final status
        code, it MAY close the transport connection or it MAY continue
        to read and discard the rest of the request.  It MUST NOT
        perform the requested method if it returns a final status code.
      - An origin server SHOULD NOT send a 100 (Continue) response if
        the request message does not include an Expect request-header
        field with the "100-continue" expectation, and MUST NOT send a
        100 (Continue) response if such a request comes from an HTTP/1.0
        (or earlier) client. There is an exception to this rule: for
        compatibility with RFC 2068, a server MAY send a 100 (Continue)
        status in response to an HTTP/1.1 PUT or POST request that does
        not include an Expect request-header field with the "100-
        continue" expectation. This exception, the purpose of which is
        to minimize any client processing delays associated with an
        undeclared wait for 100 (Continue) status, applies only to
        HTTP/1.1 requests, and not to requests with any other HTTP-
        version value.
      - An origin server MAY omit a 100 (Continue) response if it has
        already received some or all of the request body for the
        corresponding request.
      - An origin server that sends a 100 (Continue) response MUST
        ultimately send a final status code, once the request body is
        received and processed, unless it terminates the transport
        connection prematurely.
      - If an origin server receives a request that does not include an
        Expect request-header field with the "100-continue" expectation,
        the request includes a request body, and the server responds
        with a final status code before reading the entire request body
        from the transport connection, then the server SHOULD NOT close
        the transport connection until it has read the entire request,
        or until the client closes the connection. Otherwise, the client
        might not reliably receive the response message. However, this
        requirement is not be construed as preventing a server from
        defending itself against denial-of-service attacks, or from
        badly broken client implementations.

and the relevant part of RFC 7231§5.1.1:

  Requirements for servers:

   o  A server that receives a 100-continue expectation in an HTTP/1.0
      request MUST ignore that expectation.

   o  A server MAY omit sending a 100 (Continue) response if it has
      already received some or all of the message body for the
      corresponding request, or if the framing indicates that there is
      no message body.

   o  A server that sends a 100 (Continue) response MUST ultimately send
      a final status code, once the message body is received and
      processed, unless the connection is closed prematurely.

   o  A server that responds with a final status code before reading the
      entire message body SHOULD indicate in that response whether it
      intends to close the connection or continue reading and discarding
      the request message (see Section 6.6 of [RFC7230]).

   An origin server MUST, upon receiving an HTTP/1.1 (or later)
   request-line and a complete header section that contains a
   100-continue expectation and indicates a request message body will
   follow, either send an immediate response with a final status code,
   if that status can be determined by examining just the request-line
   and header fields, or send an immediate 100 (Continue) response to
   encourage the client to send the request's message body.  The origin
   server MUST NOT wait for the message body before sending the 100
   (Continue) response.

   A proxy MUST, upon receiving an HTTP/1.1 (or later) request-line and
   a complete header section that contains a 100-continue expectation
   and indicates a request message body will follow, either send an
   immediate response with a final status code, if that status can be
   determined by examining just the request-line and header fields, or
   begin forwarding the request toward the origin server by sending a
   corresponding request-line and header section to the next inbound
   server.  If the proxy believes (from configuration or past
   interaction) that the next inbound server only supports HTTP/1.0, the
   proxy MAY generate an immediate 100 (Continue) response to encourage
   the client to begin sending the message body.

      Note: The Expect header field was added after the original
      publication of HTTP/1.1 [RFC2068] as both the means to request an
      interim 100 (Continue) response and the general mechanism for
      indicating must-understand extensions.  However, the extension
      mechanism has not been used by clients and the must-understand
      requirements have not been implemented by many servers, rendering
      the extension mechanism useless.  This specification has removed
      the extension mechanism in order to simplify the definition and
      processing of 100-continue.

Nowhere does it require that the origin server do anything special to validate the headers.

[yoshuawuyts]

Do you think that this is a situation where applications might want to change the default behavior? Some apps might want to check that the request is valid before receiving the body, and some might prefer not to take the small performance hit and always immediately begin receiving the body

Ah, I was envisioning this would just work™. If sending the 100: continue response occurs when data is first requested from the body stream we cover both cases.

Re: the spec -- I've updated the title. You're right that we're currently spec-compliant, but we could be doing better.