hyper does not comply with Expect: 100-continue statement

[octplane]

I'm using hyper to receive githook commit messages and when the payload is longer than 1024 bytes, request.read_to_string() is much much slower than it probably should be (something like a factor 100 on localhost)...

I have created a very simple test case in https://github.com/octplane/githook-client-rust/tree/slow_read_hyper . It's a simple POST upload that triggers the behavior.

I suspect this might be due to the network packets but I'm not sure and I'm certain somebody here will have a much better idea on how to identify the issue :)

Thanks for your time !

[seanmonstar]

Hmm. Hyper is using a BufReader, which by default uses 64kbs. Is it significantly bigger than that?

[octplane]

It is much smaller: 1024 bytes (as indicated by the title of the Issue 😉 ). And the issue starts getting visible at precisely this value.

[seanmonstar]

Could you try using String::with_capacity(1024) or whichever the size is, and see if that affects things. i'm trying to think where in hyper it might be doing something.

[octplane]

It makes strictly no difference.

[reem]

@octplane Could you try to profile the code?

[seanmonstar]

Also, would be neat to see actual time values. 1000 bytes can be the size of packet in some cases, so it could just be doing an additional sys call to read the end.

[octplane]

@reem , @seanmonstar I have no idea on how to profile rust code... I'm on OSX, FWIW...

[octplane]

Ok, after some investigation (read the wget logs), I found out that hyper does not conform to some expected parf of the HTTP specification:

time curl -vvv -X POST -d @broken.json http://127.0.0.1:5000/hook/                                                              
* Hostname was NOT found in DNS cache
*   Trying 127.0.0.1...
* Connected to 127.0.0.1 (127.0.0.1) port 5000 (#0)
> POST /hook/ HTTP/1.1
> User-Agent: curl/7.37.1
> Host: 127.0.0.1:5000
> Accept: */*
> Content-Length: 1025
> Content-Type: application/x-www-form-urlencoded
> Expect: 100-continue
>
* Done waiting for 100-continue
< HTTP/1.1 200 OK
< Transfer-Encoding: chunked
< Date: Wed, 11 Mar 2015 20:41:28 GMT
<
* Connection #0 to host 127.0.0.1 left intact
OK.curl -vvv -X POST -d @broken.json http://127.0.0.1:5000/hook/  0,00s user 0,00s system 0% cpu 1,011 total

Because the payload is bigger than 1024 bytes, curl attempts a two step posts using the Expect: 100-continue instruction. After 1s, curl gives up and send the payload:

set->expect_100_timeout = 1000L; /* Wait for a second by default. */

https://github.com/bagder/curl/blob/df5578a7a304a23f9aa3670daff8573ec3ef416f/lib/transfer.c#L1103

Hyper happily receives the payload and call the handler.

[seanmonstar]

Thanks for finding out the real issue! To fix this, I'd imagine adding a check_continue method to the Handler trait, with a default implementation of just writing continue, and checking for an Expect header in the server loop. This allows it to just work, and allows someone to override the behavior if they need to.

[reem]

@seanmonstar I'm not convinced that hyper should actually handle this vs. downstream users/libraries adding features like this.

[seanmonstar]

@reem according to RFC7231:

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.

[reem]

I'm convinced.