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

"Connection: close" header prevents body read in certain cases #22

Open
Mzack9999 opened this issue Jul 12, 2022 · 5 comments
Open

"Connection: close" header prevents body read in certain cases #22

Mzack9999 opened this issue Jul 12, 2022 · 5 comments
Assignees
Labels
On Hold On Hold Type: Bug Inconsistencies or issues which will cause an issue or problem for users or implementors. Type: Discussion Some ideas need to be planned and disucssed to come to a strategy.

Comments

@Mzack9999
Copy link
Member

Mzack9999 commented Jul 12, 2022

retryablehttp version:

latest

Current Behavior:

When http.Transport has DisableKeepAlives: true the host https://89.28.157.20/index.action doesn't respond:

$ echo https://89.28.157.20/index.action | go run . -timeout 30 -v -silent

Expected Behavior:

$ echo https://89.28.157.20/index.action | go run . -timeout 30 -v -silent
https://89.28.157.20/index.action

Steps To Reproduce:

$ echo https://89.28.157.20/index.action | httpx -timeout 30 -v -silent

Notes

Possible approaches to minimize the occurrences:

@Mzack9999 Mzack9999 added the Type: Bug Inconsistencies or issues which will cause an issue or problem for users or implementors. label Jul 12, 2022
@ehsandeep ehsandeep added the Priority: Critical This should be dealt with ASAP. Not fixing this issue would be a serious error. label Jul 12, 2022
@Ice3man543
Copy link
Member

Ice3man543 commented Jul 22, 2022

Trying this out locally with curl -

This works

curl -v -k -H "Connection: keep-alive" https://89.28.157.20/index.action

This doesn't work. The site responds but doesn't closes the socket so curl just keeps on reading which seems non-rfc compliant.

curl -v -k -H "Connection: close" https://89.28.157.20/index.action

@Mzack9999
Copy link
Member Author

We have an error like this one:

[DBG] Failed 'https://89.28.157.20/index.action': context deadline exceeded (Client.Timeout or context cancellation while reading body)

@ehsandeep Shall we consider errors on body read as non-fatal? (If headers were read, anyway, it means there is an HTTP server)

@Mzack9999 Mzack9999 self-assigned this Jul 25, 2022
@Mzack9999 Mzack9999 added the On Hold On Hold label Jul 25, 2022
@Ice3man543
Copy link
Member

@Mzack9999 is the read body available even though the error occurs? If so, it can be okay to just ignore such body read errors. Not sure how go client behaves in such cases

@Mzack9999
Copy link
Member Author

@Ice3man543 The body is not available. Attempting to read from the request.Body reader causes the context error. The standard net/http doesn't enforce global errors, leaving it up to the user to decide how to handle the read body error exception.

@ehsandeep ehsandeep added Type: Discussion Some ideas need to be planned and disucssed to come to a strategy. and removed Priority: Critical This should be dealt with ASAP. Not fixing this issue would be a serious error. labels Aug 30, 2022
@ehsandeep
Copy link
Member

@Mzack9999 It works with curl as default, i.e not having connection header as part of the request.

curl -v -k https://89.28.157.20/index.action -XGET -sI

*   Trying 89.28.157.20:443...
* Connected to 89.28.157.20 (89.28.157.20) port 443 (#0)
* ALPN, offering h2
* ALPN, offering http/1.1
* successfully set certificate verify locations:
*  CAfile: /etc/ssl/cert.pem
*  CApath: none
* (304) (OUT), TLS handshake, Client hello (1):
* (304) (IN), TLS handshake, Server hello (2):
* TLSv1.2 (IN), TLS handshake, Certificate (11):
* TLSv1.2 (IN), TLS handshake, Server key exchange (12):
* TLSv1.2 (IN), TLS handshake, Server finished (14):
* TLSv1.2 (OUT), TLS handshake, Client key exchange (16):
* TLSv1.2 (OUT), TLS change cipher, Change cipher spec (1):
* TLSv1.2 (OUT), TLS handshake, Finished (20):
* TLSv1.2 (IN), TLS change cipher, Change cipher spec (1):
* TLSv1.2 (IN), TLS handshake, Finished (20):
* SSL connection using TLSv1.2 / ECDHE-RSA-AES128-SHA
* ALPN, server did not agree to a protocol
* Server certificate:
*  subject: CN=*.ucsp-schweiz.ch
*  start date: Mar 10 00:00:00 2022 GMT
*  expire date: Mar 15 23:59:59 2023 GMT
*  issuer: C=US; O=DigiCert Inc; CN=RapidSSL TLS DV RSA Mixed SHA256 2020 CA-1
*  SSL certificate verify ok.
> GET /index.action HTTP/1.1
> Host: 89.28.157.20
> User-Agent: curl/7.79.1
> Accept: */*
> 
* Mark bundle as not supporting multiuse
< HTTP/1.1 200 OK
HTTP/1.1 200 OK
< Server: Apache-Coyote/1.1
Server: Apache-Coyote/1.1
< Pragma: no-cache
Pragma: no-cache
< Cache-Control: no-cache, no-store
Cache-Control: no-cache, no-store
< Expires: -1
Expires: -1
< X-Powered-By: 
X-Powered-By: 
< Strict-Transport-Security: max-age=31536000; includeSubDomains
Strict-Transport-Security: max-age=31536000; includeSubDomains
< X-Frame-Options: SAMEORIGIN
X-Frame-Options: SAMEORIGIN
< Set-Cookie: JSESSIONID=28AF00B872D1D28966AC4A8D0C7DDD8C; Path=/; Secure; HttpOnly
Set-Cookie: JSESSIONID=28AF00B872D1D28966AC4A8D0C7DDD8C; Path=/; Secure; HttpOnly
< Set-Cookie: JSESSIONID=43BFF33572E1C76E296E29762677F831; Path=/; Secure; HttpOnly
Set-Cookie: JSESSIONID=43BFF33572E1C76E296E29762677F831; Path=/; Secure; HttpOnly
< Content-Type: text/html;charset=utf-8
Content-Type: text/html;charset=utf-8
< Transfer-Encoding: chunked
Transfer-Encoding: chunked
< Date: Tue, 30 Aug 2022 10:46:31 GMT
Date: Tue, 30 Aug 2022 10:46:31 GMT

< 
* Connection #0 to host 89.28.157.20 left intact

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
On Hold On Hold Type: Bug Inconsistencies or issues which will cause an issue or problem for users or implementors. Type: Discussion Some ideas need to be planned and disucssed to come to a strategy.
Projects
None yet
Development

No branches or pull requests

3 participants