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.

Sign up for GitHub

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

Jump to bottom

HTTP/2 request content upload stalls when response headers are received #10433

Closed
LPardue opened this issue Feb 7, 2023 · 3 comments
Closed

HTTP/2 request content upload stalls when response headers are received #10433

LPardue opened this issue Feb 7, 2023 · 3 comments
Assignees
@icing
Labels
HTTP/2

Comments

@LPardue
Copy link
Contributor

LPardue commented Feb 7, 2023 •
edited
Loading

I did this

Scenario: a curl client POSTing a large file (say over 100 KB) to an HTTP/2 server that is configured to respond immediately before a) waiting for all of the indicated content-length to be received or b) waiting for the request stream FIN. Note that this server also leaves the response side open until it detects the client closes its side.

I'm aware of reports that in such a scenario, once curl receives the response, the upload no longer progresses. For servers that have some kind of body read timeout (e.g. http://nginx.org/en/docs/http/ngx_http_proxy_module.html#proxy_send_timeout) this means that the stream or connection is eventually timed and reset or closed with an error

I expected the following

When a response is received before curl completes a request upload, and the server has not closed its side, curl should continue the upload.

curl/libcurl version

One confirmed report with an old verion

curl 7.74.0 (x86_64-pc-linux-gnu) libcurl/7.74.0 OpenSSL/1.1.1n zlib/1.2.11 brotli/1.0.9 libidn2/2.3.0 libpsl/0.21.0 (+libidn2/2.3.0) libssh2/1.9.0 nghttp2/1.43.0 librtmp/2.3
Release-Date: 2020-12-09

Uncomfirmed reports with later versions exhibit the same behaviour.

operating system

Debian 5.10.158-2
@bagder bagder added the HTTP/2 label Feb 7, 2023
@bagder
Copy link
Member

bagder commented Feb 7, 2023

I believe @icing has a fix for this?

@icing icing self-assigned this Feb 7, 2023
@icing
Copy link
Contributor

icing commented Feb 7, 2023

I have indeed. Will make a PR tomorrow pbly.

icing added a commit to icing/curl that referenced this issue Feb 8, 2023
@icing
HTTP/2 and HTTP/3 upload handling, continue upload when state.drain i…
a8b145b 
…s set.

- as reported in curl#10433, HTTP/2 uploads may stall when a response is
  received before the upload is done. This happens when the data->state.drain
  is set for such a transfer, as the special handling in transfer.c from
  then on only cared about downloads.
- add continuation of uploads, if applicable, in this case.
- add pytest case test_07_12_upload_seq_large to reproduce this scenario
  (although, current nghttp2 implementation is using drain less often)
@icing icing mentioned this issue Feb 8, 2023
Closed
@icing
Copy link
Contributor

icing commented Feb 8, 2023

Fix and test case for this in #10443.

@bagder bagder closed this as completed in 3de3ea6 Feb 9, 2023
bch pushed a commit to bch/curl that referenced this issue Jul 19, 2023
@icing @bch
HTTP/[23]: continue upload when state.drain is set
71733cf 
- as reported in curl#10433, HTTP/2 uploads may stall when a response is
  received before the upload is done. This happens when the
  data->state.drain is set for such a transfer, as the special handling
  in transfer.c from then on only cared about downloads.
- add continuation of uploads, if applicable, in this case.
- add pytest case test_07_12_upload_seq_large to reproduce this scenario
  (although, current nghttp2 implementation is using drain less often)

Reported-by: Lucas Pardue

Fixes curl#10433
Closes curl#10443
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@icing icing

Labels
HTTP/2
Milestone
No milestone
Development

No branches or pull requests
3 participants
@icing @bagder @LPardue