ducaale / xh

Friendly and fast tool for sending HTTP requests
MIT License
4.96k stars 90 forks source link

hyper v1 upgrade #357

Closed zuisong closed 3 months ago

zuisong commented 3 months ago

two test cases that cannot pass, I have already submitted an issue to reqwest. https://github.com/seanmonstar/reqwest/issues/2201

blyxxyz commented 3 months ago

xh does send Content-Length now even for GET requests. That seems weird since RFC 9110 says

A user agent SHOULD NOT send a Content-Length header field when the request message does not contain content and the method semantics do not anticipate such data.

It seems to be a bug? See https://github.com/seanmonstar/reqwest/issues/2202#issuecomment-2016577201. Notably this is causing real-world issues for someone, so we shouldn't send this header for GET requests if we can avoid it.

Otherwise we'd have to add it to the request headers that we print.

Test case Run this in one terminal: ```sh while nc -l -p 8080; do echo; done ``` Then run this in another: ```sh cargo run -- -v :8080 ``` This is what xh claims to send: ``` GET / HTTP/1.1 Accept: */* Accept-Encoding: gzip, deflate, br Connection: keep-alive Host: localhost:8080 User-Agent: xh/0.21.0 ``` This is what's actually received: ``` GET / HTTP/1.1 Accept-Encoding: gzip, deflate, br User-Agent: xh/0.21.0 Connection: keep-alive Accept: */* Host: localhost:8080 Content-Length: 0 ```
zuisong commented 3 months ago

Thanks for pointing that out,I've submitted a PR to fix it. https://github.com/seanmonstar/reqwest/pull/2207

blyxxyz commented 3 months ago

This is very odd, but with your patched reqwest xh seems to send a body of 0 by default, i.e. the text 0. This is what I receive through netcat from xh post :8080:

POST / HTTP/1.1
Accept-Encoding: gzip, deflate, br
User-Agent: xh/0.21.0
Connection: keep-alive
Accept: */*
Host: localhost:8080
Transfer-Encoding: chunked

0

(or escaped: POST / HTTP/1.1\r\nAccept-Encoding: gzip, deflate, br\r\nUser-Agent: xh/0.21.0\r\nConnection: keep-alive\r\nAccept: */*\r\nHost: localhost:8080\r\nTransfer-Encoding: chunked\r\n\r\n0\r\n\r\n)

It doesn't happen if I comment out the reqwest patch in Cargo.toml.

(EDIT: this happens because reqwest chooses to use chunked transfer encoding.)


I've also noticed that with hyper v1 xh now errors if you override the Content-Length to be too large, e.g. xh post :8080 Content-Length:10. I don't think that's a big deal though, definitely not blocking for this PR.

zuisong commented 3 months ago

It's really weird. I'll keep looking for a solution.

zuisong commented 3 months ago

The issue is fixed now, could you check if there are other issues?

zuisong commented 3 months ago

Reqwest just released a new version 0.12.3, so this PR can be ready to go