-
-
Notifications
You must be signed in to change notification settings - Fork 6.6k
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
HTTP/2 stream is not reset after timeout #2882
Comments
I'm afraid this doesn't seem to reproduce that easily for me. In my log output I get (after ~10 seconds):
|
@mkauf, can you try to set a break-point in |
@bagder Yes, It works for me when I add a call to (Maybe you can reproduce the bug if you increase the buffer size in the PHP script. PHP does buffering, Apache too... so sending more bytes could help) |
Thanks, now I understand it much better! |
I think calling I believe the larger issue is harder and might take something like #1641 to get fixed properly, since we will need to monitor this connection after the transfer is complete on it and it is returned to the connection pool, in an edge case to make sure the |
|
Huh? This is a http2 connection so I don't understand that comment. We use |
And by all means, make a PR out of it! 👍 |
|
Oh, right. So I think it should better bail out early if so: diff --git a/lib/http2.c b/lib/http2.c
index e9ec73a6d..1031d0627 100644
--- a/lib/http2.c
+++ b/lib/http2.c
@@ -1144,10 +1144,13 @@ void Curl_http2_done(struct connectdata *conn, bool premature)
{
struct Curl_easy *data = conn->data;
struct HTTP *http = data->req.protop;
struct http_conn *httpc = &conn->proto.httpc;
+ if(!httpc->h2) /* not HTTP/2 ? */
+ return;
+
if(data->state.drain)
drained_transfer(data, httpc);
if(http->header_recvbuf) {
Curl_add_buffer_free(http->header_recvbuf); |
If this is the last stream on this connection, the RST_STREAM might not get pushed to the wire otherwise. Fixes #2882 Researched-by: Michael Kaufmann
I hope you don't mind, but I created #2887 to show my take. What do you say? |
Thanks, I left comments in the PR. |
If this is the last stream on this connection, the RST_STREAM might not get pushed to the wire otherwise. Fixes #2882 Researched-by: Michael Kaufmann
I did this
The following test program creates two transfers:
After the timeout of 10 seconds, the first transfer (HTTP/2) should stop and the HTTP/2 stream should be reset. libcurl does not deliver the received data anymore to the application, but libcurl does not reset the HTTP/2 stream and the data is still transferred over the wire. This can be checked with Wireshark (hints). As long as the second transfer (HTTP/1.1) is active, the HTTP/2 transfer is not really stopped.
The URL points to a PHP script that sends data in an infinite loop:
Test program:
I expected the following
When the HTTP/2 transfer timeouts, the HTTP/2 stream should be reset.
curl/libcurl version
curl 7.61.1-DEV (x86_64-pc-linux-gnu) libcurl/7.61.1-DEV OpenSSL/1.1.0h zlib/1.2.11 nghttp2/1.32.0
Release-Date: [unreleased]
Protocols: dict file ftp ftps gopher http https imap imaps pop3 pop3s rtsp smb smbs smtp smtps telnet tftp
Features: AsynchDNS IPv6 Largefile NTLM NTLM_WB SSL libz TLS-SRP HTTP2 UnixSockets HTTPS-proxy
operating system
Linux
The text was updated successfully, but these errors were encountered: