Expect Continue

[slava-vishnyakov]

When curl sends a long POST body, it sends Expect: 100-continue header.

Content-Type: application/json
Content-Length: 4641
Expect: 100-continue

Without this patch - curl will wait for 1 second before sending the body (which is unreasonable delay).

It's easy to test - just curl -XPOST -d'(a few thousand bytes here)' and you will see the delay.

[WyriHaximus]

Looks good, can you add/update tests? 👍

[slava-vishnyakov]

Thanks! Test added.

[slava-vishnyakov]

Any chance to get this merged?

[WyriHaximus]

Hah sorry, missed your previous message. Looks good merging it 👍

[clue]

Thanks for your PR @slava-vishnyakov, I very much appreciate your contribution 👍

However, we're currently looking into reverting this PR as part of #59 because it introduces a BC break.

As per https://tools.ietf.org/html/rfc7231#section-5.1.1, HTTP clients are expected to support servers not handling the Expect: 100-continue header at all. This means that curl does it fact behave correctly by waiting for a small period of time and then continuing like normal.

This can currently be avoided like this:

$http->on('request', function ($request, $response) {
    if ($request->expectsContinue()) {
        $response->writeContinue().
    }
    $response->writeHead(200, array('Content-Type' => 'text/plain'));
    $response->end("Hello World!\n");
});

This is necessary because this block of code could implement some custom application logic to check the request headers and such and we can't really provide a "default implementation" here. However, this feature could certainly use some documentation!

I hope this doesn't come over as discouraging and we can agree that it makes sense to revert this until we can agree on a better solution here 👍

[slava-vishnyakov]

@clue Hi Christian, sure, do what's best for the project. Although surely you understand that reverting this is also a BC for all customers who already use this :) I mean, libcurl is probably the most popular implementation of HTTP client. Maybe a better way would be to add a configuration option somewhere as to whether use this logic or not?

Otherwise, sure, do what's best for the project 👍