Expect header not handled properly

[daguej]

When a HTTP client makes a request with a request-body (POST or PUT) and sends an Expect header with a value of 100-continue, origin servers MUST respond with either a HTTP 100 response and then continue to read the input stream, or respond with an error. If the client supplies any other value for Expect that the server does not understand, the server MUST respond with a 417 error and stop processing the request.

The underlying Node HTTP module raises a checkContinue event when it sees Expect: 100-continue in the completed headers, and before request is fired. If there are no event handlers listening to the event, Node automatically responds with 100 Continue and then fires the request event. Since Connect does not register a checkContinue handler, the Continue response is sent, and then request is fired.

Under normal conditions, this works OK -- request headers come in, Node sends a 100 for us, and then the client sends its request body. From Connect's standpoint, it was just a normal request.

However, there are certain situations where this breaks:

• If a client sends an unknown token in the Expect header, it is silently ignored, in violation of the spec.

  A server that does not understand or is unable to comply with any of the expectation values in the Expect field of a request MUST respond with appropriate error status. The server MUST respond with a 417 (Expectation Failed) status if any of the expectations cannot be met or, if there are other problems with the request, some other 4xx status.

  Application authors should not have to manually check the Expect header (ie, explicitly use some middleware function) just to properly handle a protocol detail. Of course, the flip side of this is that application authors should be able to handle custom Expect values.

  I suspect this particular situation is actually something that should be handled in core (the http module), so I've submitted an issue.

• Application authors should have the ability to preprocess requests and respond appropriately to the expectation. For example, if my server requires HTTP authentication, this happens:

  ​> POST /upload HTTP/1.1
  ​> Expect: 100-continue
  ​> Content-Length: 104857600
  
  < HTTP/1.1 100 Continue
  
  ​> ... (100 MB)
  
  < HTTP/1.1 401 Unauthorized
  < WWW-Authenticate: Basic realm="test"
  
  ​> POST /upload HTTP/1.1
  ​> Expect: 100-continue
  ​> Authorization: Basic ...
  ​> Content-Length: 104857600
  
  < HTTP/1.1 100 Continue
  
  ​> ... (100 MB)
  
  < HTTP/1.1 200 OK
  < ...
  

  If the application had the opportunity to respond to the expectation (the checkContinue event), I could have told the client it needed to authenticate before it wasted 100 MB of transfer.

    ​> POST /upload HTTP/1.1
    ​> Expect: 100-continue
    ​> Content-Length: 104857600
  
    < HTTP/1.1 401 Unauthorized
    < WWW-Authenticate: Basic realm="test"
  
    ​> POST /upload HTTP/1.1
    ​> Expect: 100-continue
    ​> Authorization: Basic ...
    ​> Content-Length: 104857600
  
    < HTTP/1.1 100 Continue
  
    ​> ... (100 MB)
  
    < HTTP/1.1 200 OK
    < ...
  

Connect should expose some kind of mechanism that would allow an application to handle a continue expectation.

[tj]

yeah agreed, the way node handles it is kinda awkward IMO, I'll see what I can do

[tj]

come to think of it, there's little we can do directly since Connect/Express applications are now simply callback functions, they don't really operate on the node http server at all. With node having no concept of response absence other than a timeout I guess it doesn't really add a flag either. You would have to do something like this ATM:

var server = app.listen(3000);

server.on('checkContinue', function(req, res){
  req.checkContinue = true;
  app(req, res);
});

[aseemk]

Running into this myself -- would be great to have integrated support for 100-continue. The workaround will work technically -- just a bit tricky w.r.t. code organization and logic. Thanks though TJ.

[aseemk]

Conceptually, it feels like all middleware (except bodyParser, of course), and even routing, can happen based on just the headers alone. AFAICT, I only ever reference parsed bodies in my routes.

So just thinking out loud, it'd be awesome if a future version of Connect/Express were designed to listen to the checkContinue event by default, and the bodyParser middleware changed so you could "pull" the body when you wanted to use it, and it would automatically call writeContinue() then if it needed to.

[tj]

that will be a lot easier with the next thing I'm working on based on generators, current node stuff isn't very good at pull-based anything, you'd have to have a callback like req.body(fn) but that's lame

[jonathanong]

we'll revisit this after node v0.12. reopen with suggestions.

kind of against this since it will require connect to wrap around the http server, and that's something i'd much rather do myself

[uiteoi]

On way to handle this without having connect wrapping around the http server, would be to have developers redirect explicitly the 'checkContinue' event to a Connect application:

server.on( 'checkContinue', app )

Then have some middleware handle the Expect header.

This would work since http events 'request' and 'checkContinue' have the same signature function( request, response).