Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

curl should repeat a request without a 100-continue expectation when receiving an HTTP 417 (Expectation Failed) status code #4949

Closed
bramus opened this issue Feb 18, 2020 · 4 comments
Labels

Comments

@bramus
Copy link

@bramus bramus commented Feb 18, 2020

I did this

  1. Invoke a POST request with over 1024 bytes of data
curl -X POST -d "…" http://example.com/
  1. On the server, I received this:
POST / HTTP/1.1
Host: localhost:8080
User-Agent: curl/7.64.1
Accept: */*
Content-Length: 1054
Content-Type: application/x-www-form-urlencoded
Expect: 100-continue


(Nice of you curl, to warn the server about a "huge" incoming payload 😊)

  1. On the server – which I am building – I send back an HTTP 417 and keep the connection open
HTTP/1.1 417 Expectation Failed

I expected the following

I expected curl to repeat the entire request without the Expect: 100-continue header, as per HTTP specification:

A client that receives a 417 (Expectation Failed) status code in
response to a request containing a 100-continue expectation SHOULD
repeat that request without a 100-continue expectation, since the
417 response merely indicates that the response chain does not
support expectations (e.g., it passes through an HTTP/1.0 server).

Instead curl just sent the data over the connection, as if it had received an HTTP/1.1 100 Continue.

curl/libcurl version

$ curl -V
curl 7.64.1 (x86_64-apple-darwin19.0) libcurl/7.64.1 (SecureTransport) LibreSSL/2.8.3 zlib/1.2.11 nghttp2/1.39.2
Release-Date: 2019-03-27
Protocols: dict file ftp ftps gopher http https imap imaps ldap ldaps pop3 pop3s rtsp smb smbs smtp smtps telnet tftp 
Features: AsynchDNS GSS-API HTTP2 HTTPS-proxy IPv6 Kerberos Largefile libz MultiSSL NTLM NTLM_WB SPNEGO SSL UnixSockets

operating system

macOS 10.15.2 (19C57)

@bramus
Copy link
Author

@bramus bramus commented Feb 18, 2020

Digging deeper I notice that curl sends over the post body no matter what the server does. The request is sent in two parts (headers + body), unless the server has already closed the connection after having received the headers.

@bagder bagder added the HTTP label Feb 19, 2020
@bagder
Copy link
Member

@bagder bagder commented Feb 19, 2020

This is indeed a bug. 417 is just a very unusual response code so we've never had reason to implement this properly before.

Two additional comments:

Expect: 100 doesn't imply that there's a "huge" payload coming and the size limit when curl adds the header has been increased since the version you use

curl sends over the post body no matter what the server does

If it does, it is a (separate) bug.

@bramus
Copy link
Author

@bramus bramus commented Feb 19, 2020

Regarding “curl sends over the post body no matter what the server does”, I found this in the spec so it's allowed:

A client that sends a 100-continue expectation is not required to
wait for any specific length of time; such a client MAY proceed to
send the message body even if it has not yet received a response.
Furthermore, since 100 (Continue) responses cannot be sent through
an HTTP/1.0 intermediary, such a client SHOULD NOT wait for an
indefinite period before sending the message body.

By default curl waits 1 second. The timeout is configurable through the --expect100-timeout option.

@bagder
Copy link
Member

@bagder bagder commented Feb 19, 2020

That's not "no matter what the server does" though. That's when the server doesn't provide 100 within the timeout. And if the server returns an error instead, curl shouldn't send the body either.

bagder added a commit that referenced this issue Feb 21, 2020
When doind a request with a body + Expect: 100-continue and the server
responds with a 417, the same request will be retried immediately
without the Expect: header.

Added test 357 to verify.

Also added a control instruction to tell the sws test server to not read
the request body if Expect: is present, which the new test 357 uses.

Fixes #4949
bagder added a commit that referenced this issue Feb 21, 2020
When doing a request with a body + Expect: 100-continue and the server
responds with a 417, the same request will be retried immediately
without the Expect: header.

Added test 357 to verify.

Also added a control instruction to tell the sws test server to not read
the request body if Expect: is present, which the new test 357 uses.

Reported-by: bramus on github
Fixes #4949
@bagder bagder closed this in 6375b20 Feb 26, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

2 participants
You can’t perform that action at this time.