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

hyper: root cause of curl test 265 failure #8853

Closed
pmk21 opened this issue May 15, 2022 · 6 comments
Closed

hyper: root cause of curl test 265 failure #8853

pmk21 opened this issue May 15, 2022 · 6 comments
Assignees

Comments

@pmk21
Copy link
Contributor

pmk21 commented May 15, 2022

I did this

Ran curl test 265, analyzed the debug trace and generated logs, concluded that hyper is probably closing the connection on receiving a HTTP/1.0 response.

I switched HTTP/1.0 with HTTP/1.1 at these locations -

HTTP/1.0 407 Authorization Required to proxy me my dear

HTTP/1.0 407 Authorization Required to proxy me my dear

The test seems to be passing, though there are a few small differences in the output -

265: protocol FAILED:
--- log/check-expected  2022-05-14 16:27:59.457379329 +0530
+++ log/check-generated 2022-05-14 16:27:59.457379329 +0530
@@ -1,16 +1,16 @@
-CONNECT test.remote.example.com.265:44167 HTTP/1.1[LF]
+CONNECT test.remote.example.com.265:44167 HTTP/1.1[CR][LF]
 Host: test.remote.example.com.265:44167[CR][LF]
 Proxy-Authorization: NTLM TlRMTVNTUAABAAAABoIIAAAAAAAAAAAAAAAAAAAAAAA=[CR][LF]
 User-Agent: curl/7.83.1-DEV[CR][LF]
 Proxy-Connection: Keep-Alive[CR][LF]
 [CR][LF]
-CONNECT test.remote.example.com.265:44167 HTTP/1.1[LF]
+CONNECT test.remote.example.com.265:44167 HTTP/1.1[CR][LF]
 Host: test.remote.example.com.265:44167[CR][LF]
 Proxy-Authorization: NTLM TlRMTVNTUAADAAAAGAAYAEAAAAAYABgAWAAAAAAAAABwAAAACAAIAHAAAAAIAAgAeAAAAAAAAAAAAAAAhoIBAFpkQwKRCZFMhjj0tw47wEjKHRHlvzfxQamFcheMuv8v+xeqphEO5V41xRd7R9deOXRlc3R1c2VyY3VybGhvc3Q=[CR][LF]
 User-Agent: curl/7.83.1-DEV[CR][LF]
 Proxy-Connection: Keep-Alive[CR][LF]
 [CR][LF]
-POST /path/2650002 HTTP/1.1[LF]
+POST /path/2650002 HTTP/1.1[CR][LF]
 Host: test.remote.example.com.265:44167[CR][LF]
 User-Agent: curl/7.83.1-DEV[CR][LF]
 Accept: */*[CR][LF]

I expected the following

After switching to HTTP/1.1, I expected the test case to pass, but there are a few small differences.

curl/libcurl version

curl 7.83.1-DEV (x86_64-pc-linux-gnu) libcurl/7.83.1-DEV OpenSSL/1.1.1o zlib/1.2.12 brotli/1.0.9 zstd/1.5.2 libidn2/2.3.2 libpsl/0.21.1 (+libidn2/2.3.0) librtmp/2.3 Hyper/0.14.18 OpenLDAP/2.6.1
Release-Date: [unreleased]
Protocols: dict file ftp ftps gopher gophers http https imap imaps ldap ldaps mqtt pop3 pop3s rtmp smb smbs smtp smtps telnet tftp 
Features: alt-svc AsynchDNS brotli Debug HSTS HTTP2 HTTPS-proxy IDN Largefile libz NTLM NTLM_WB PSL SSL TLS-SRP TrackMemory UnixSockets zstd

operating system

Linux msi 5.15.38-1-MANJARO #1 SMP PREEMPT Mon May 9 07:52:21 UTC 2022 x86_64 GNU/Linux
@DemiMarie
Copy link

@pmk21 test case changes look correct, question is if this is a hyper bug.

@pmk21
Copy link
Contributor Author

pmk21 commented Nov 9, 2022

Not sure. I guess hyper is very strict about the HTTP version. Is a change in HTTP version during a transaction valid?

@DemiMarie
Copy link

Possibly? I suggest filing a Hyper ticket.

@bagder
Copy link
Member

bagder commented Dec 8, 2022

@seanmonstar any comment on this? Does Hyper not like when the proxy is a HTTP/1.0 one and returns that in the response?

Test 265 works with hyper for me when I apply this patch:

diff --git a/tests/data/test265 b/tests/data/test265
index c6db8964e..56e89ba53 100644
--- a/tests/data/test265
+++ b/tests/data/test265
@@ -13,11 +13,11 @@ NTLM
 # Server-side
 <reply>
 
 # this is returned first since we get no proxy-auth
 <connect1001>
-HTTP/1.0 407 Authorization Required to proxy me my dear
+HTTP/1.1 407 Authorization Required to proxy me my dear^M
 Proxy-Authenticate: NTLM TlRMTVNTUAACAAAAAgACADAAAACGggEAc51AYVDgyNcAAAAAAAAAAG4AbgAyAAAAQ0MCAAQAQwBDAAEAEgBFAEwASQBTAEEAQgBFAFQASAAEABgAYwBjAC4AaQBjAGUAZABlAHYALgBuAHUAAwAsAGUAbABpAHMAYQBiAGUAdABoAC4AYwBjAC4AaQBjAGUAZABlAHYALgBuAHUAAAAAAA==
 Content-Length: 1033
 
 And you should ignore this data.
 QQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQ
@@ -51,11 +51,11 @@ Server: no
 
 Nice proxy auth sir!
 </data1000>
 
 <datacheck>
-HTTP/1.0 407 Authorization Required to proxy me my dear
+HTTP/1.1 407 Authorization Required to proxy me my dear^M
 Proxy-Authenticate: NTLM TlRMTVNTUAACAAAAAgACADAAAACGggEAc51AYVDgyNcAAAAAAAAAAG4AbgAyAAAAQ0MCAAQAQwBDAAEAEgBFAEwASQBTAEEAQgBFAFQASAAEABgAYwBjAC4AaQBjAGUAZABlAHYALgBuAHUAAwAsAGUAbABpAHMAYQBiAGUAdABoAC4AYwBjAC4AaQBjAGUAZABlAHYALgBuAHUAAAAAAA==
 Content-Length: 1033
 
 HTTP/1.1 200 Things are fine in proxy land
 Server: Microsoft-IIS/5.0

@seanmonstar
Copy link
Contributor

I was a little confused, so I set up a Rust example doing the same thing, logs to max, and now I know what the deal is: hyper sees a 1.0 response without a Connection: keep-alive, and thus sets its internal state to "cannot reuse", whereas the default for a 1.1 response with no Connection header is to be able to reuse it.

@bagder
Copy link
Member

bagder commented Dec 8, 2022

Aha, I think we can live with that and just make the test server return that header.

The way I recall things this test originates from 2005 and is more or less a copy of a real world case of a proxy doing NTLM with HTTP/1.0 like this. It might not be terribly important.

When I fix this, I get another issue that looks a curl bug in the hyper related adaptions. I'll work on it.

bagder added a commit that referenced this issue Dec 8, 2022
Together with d31915a it makes test 265 run fine.

Fixes #8853
Assisted-by: Prithvi MK
Assisted-by: Sean McArthur
@bagder bagder closed this as completed in c8d24d4 Dec 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

4 participants