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

HTTP/2 infinite retries on REFUSED_STREAM #5794

Closed
Cherish98 opened this issue Aug 7, 2020 · 4 comments
Closed

HTTP/2 infinite retries on REFUSED_STREAM #5794

Cherish98 opened this issue Aug 7, 2020 · 4 comments
Labels

Comments

@Cherish98
Copy link
Contributor

@Cherish98 Cherish98 commented Aug 7, 2020

I did this

I set up a (misbehaving) HTTP/2 server that always returns REFUSED_STREAM. Then, I ran curl to make a request. curl retried infinitely.

The HTTP/2 server was set up with haproxy, which always returns REFUSED_STREAM (edit: that is the intended behaviour, when the upstream server closes the TCP connection without sending any data; previously I thought that was a bug on their end). The minimum haproxy configuration file that can reproduce the bug (reproducible on haproxy 2.0.17, 2.1.7, and 2.2.2, but not < 1.9):

defaults
    timeout connect 5s
    timeout client 1m
    timeout server 1m

listen bad
    mode tcp
    bind *:1234

listen https
    mode http
    bind *:443 ssl crt /tmp/test.pem alpn h2
    server bad 127.0.0.1:1234

The curl command line is simply curl -vk https://127.0.0.1

I expected the following

curl should not retry at all, because I did not set the --retry option.

Or, it should retry a limited number of times, as intended in #5074. The fix in #5074 didn't work because on every retry, the old connection is freed and a new one is created, and thus conn->retrycount is always zero.

This bug is probably related to #5250.

curl/libcurl version

(git master)
curl 7.72.0-DEV (x86_64-pc-linux-gnu) libcurl/7.72.0-DEV OpenSSL/1.1.1g zlib/1.2.11 zstd/1.4.5 libidn2/2.3.0 libpsl/0.21.0 (+libidn2/2.2.0) nghttp2/1.41.0

operating system

Linux Arch 5.4.50-1-lts #1 SMP Wed, 01 Jul 2020 14:53:03 +0000 x86_64 GNU/Linux

curl -v output

* STATE: PROTOCONNECT => DO handle 0x55fe6520aa68; line 1900 (connection #62)
* Using HTTP2, server supports multi-use
* Connection state changed (HTTP/2 confirmed)
* Copying HTTP/2 data in stream buffer to connection buffer after upgrade: len=0
* Using Stream ID: 1 (easy handle 0x55fe6520aa68)
> GET / HTTP/2
> Host: 127.0.0.1
> user-agent: curl/7.72.0-DEV
> accept: */*
>
* STATE: DO => DO_DONE handle 0x55fe6520aa68; line 1955 (connection #62)
* multi changed, check CONNECT_PEND queue!
* STATE: DO_DONE => PERFORM handle 0x55fe6520aa68; line 2076 (connection #62)
* TLSv1.3 (IN), TLS handshake, Newsession Ticket (4):
* old SSL session ID is stale, removing
* Connection state changed (MAX_CONCURRENT_STREAMS == 100)!
* multi changed, check CONNECT_PEND queue!
* Marked for [closure]: REFUSED_STREAM
* REFUSED_STREAM, retrying a fresh connect
* Connection died, retrying a fresh connect
* multi_done
* The cache now contains 0 members
* Closing connection 62
* TLSv1.3 (OUT), TLS alert, close notify (256):
* Issue another request to this URL: 'https://127.0.0.1/'
* STATE: PERFORM => CONNECT handle 0x55fe6520aa68; line 2243 (connection #-5000)
* Added connection 63. The cache now contains 1 members
* Hostname 127.0.0.1 was found in DNS cache
* family0 == v4, family1 == v6
*   Trying 127.0.0.1:443...
* STATE: CONNECT => WAITCONNECT handle 0x55fe6520aa68; line 1730 (connection #63)
* Connected to 127.0.0.1 (127.0.0.1) port 443 (#63)
@bagder bagder added the HTTP/2 label Aug 7, 2020
@bagder
Copy link
Member

@bagder bagder commented Aug 8, 2020

Clearly #5074 was incomplete and it needs to be fixed.

@StefanYohansson
Copy link
Contributor

@StefanYohansson StefanYohansson commented Aug 9, 2020

I am trying to fix this issue... this is what I am using to reproduce the problem as suggested by @Cherish98

for someone who needs an env to reproduce the issue later.

https://gist.github.com/StefanYohansson/ce450ef0ef2020a3fd5837da593cfcc2

@bagder
Copy link
Member

@bagder bagder commented Aug 10, 2020

@StefanYohansson awesome! A first shot should probably just move retrycount from the connection struct to the Curl_easy struct so that it properly counts connection attempts per-transfer.

@StefanYohansson
Copy link
Contributor

@StefanYohansson StefanYohansson commented Aug 10, 2020

I have a fix now: #5800
I don't know if semantically it is right accordingly with the project structure but it works. If you guys have any thoughts on that.

EDIT: it is exactly what you suggested @bagder :)

@bagder bagder linked a pull request that will close this issue Aug 10, 2020
@bagder bagder closed this in 50dd05a Aug 10, 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.

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