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

Redirected requests don't pass on HTTP Headers set on the originating request #1990

Closed
RickMoynihan opened this issue Oct 2, 2023 · 3 comments · Fixed by #2256
Closed

Redirected requests don't pass on HTTP Headers set on the originating request #1990

RickMoynihan opened this issue Oct 2, 2023 · 3 comments · Fixed by #2256
Assignees
Labels
bug Something isn't working
Milestone

Comments

@RickMoynihan
Copy link

Firstly thanks a million for Hurl, it's great! ❤️

What is the current bug behavior?

My understanding is that hurl Options claim to have the same semantics as curl

Options that exist in curl have exactly the same semantics.
-- Options

However as we'll see in this issue, the semantics between hurl and curl differ, which means hurl can get a different result to what curl would get in the presence of a redirect (302).

Steps to reproduce

  • Have a server that 302 redirects an incoming GET request to a different HTTP resource, where that second resources response is dependent on HTTP headers in the original request, e.g. Accept:

  • Use the following hurl file:

GET http://localhost:3000/original-resource
Accept: text/csv
[Options]
location: true

HTTP 200

Which when run (depending on the server) results in verbose mode output like this:

* Options:
*     fail fast: true
*     follow redirect: true
*     insecure: false
*     max redirect: 50
*     retry: 0
* Variables:
*     release: 2023
*     series: mixup-0003
* ------------------------------------------------------------------------------
* Executing entry 1
*
* Entry options:
* location: true
*
* Cookie store:
*
* Request:
* GET http://localhost:3000/original-resource
* Accept: text/csv
*
* Request can be run with the following curl command:
* curl --header 'Accept: text/csv' --location 'http://localhost:3000/original-resource'
*
> GET /original-resource HTTP/1.1
> Host: localhost:3000
> Accept: text/csv
> User-Agent: hurl/4.0.0
>
* Response: (received 0 bytes in 63 ms)
*
< HTTP/1.1 302 Found
< Date: Mon, 02 Oct 2023 10:30:30 GMT
< Location: /second-resource
< Transfer-Encoding: chunked
< Server: Jetty(9.4.38.v20210224)
<
*
* => Redirect to http://localhost:3000/second-resource
*
> GET /second-resource
> Host: localhost:3000
> Accept: */*
> User-Agent: hurl/4.0.0
>
* Response: (received 934 bytes in 7 ms)
*
< HTTP/1.1 200 OK
< Date: Mon, 02 Oct 2023 10:30:30 GMT
< Content-Type: application/json
< Transfer-Encoding: chunked
< Server: Jetty(9.4.38.v20210224)
< <JSON_OUTPUT_HERE>
*

NOTE here the server responding with a default Content-Type of application/json because the Accept: text/csv was not propogated onto the /second-resource request (and was instead left as Accept: */*).

What is the expected correct behavior?

The expected behaviour is that the second request following the redirect should include the specified Accept: text/csv header. We can see that curl's behaviour does this by running the equivalent curl command hurl suggests and appending a -v:

$ curl --header 'Accept: text/csv' --location 'http://localhost:3000/original-resource' -v
*   Trying 127.0.0.1:3000...
* Connected to localhost (127.0.0.1) port 3000 (#0)
> GET /original-resource HTTP/1.1
> Host: localhost:3000
> User-Agent: curl/7.88.1
> Accept: text/csv
> 
< HTTP/1.1 302 Found
< Date: Mon, 02 Oct 2023 10:45:57 GMT
< Location: /second-resource
< Transfer-Encoding: chunked
< Server: Jetty(9.4.38.v20210224)
< 
* Ignoring the response-body
* Connection #0 to host localhost left intact
* Issue another request to this URL: 'http://localhost:3000/second-resource'
* Found bundle for host: 0x60000352c8a0 [serially]
* Can not multiplex, even if we wanted to
* Re-using existing connection #0 with host localhost
> GET /second-resource HTTP/1.1
> Host: localhost:3000
> User-Agent: curl/7.88.1
> Accept: text/csv
> 
< HTTP/1.1 200 OK
< Date: Mon, 02 Oct 2023 10:45:57 GMT
< Content-Type: text/csv
< Server: Jetty(9.4.38.v20210224)
< 
csv,output,here
1,2,3
* Connection #0 to host localhost left intact

Additionally the behaviour is described in the curl manual...e.g. under -H http headers there is this note:

WARNING: headers set with this option are set in all HTTP requests - even after redirects are followed, like when told with -L, --location. This can lead to the header being sent to other hosts than the original host, so sensitive headers should be used with caution combined with following redirects.

Execution context

Apple Mac O/S Ventura 13.4.1:

hurl 4.0.0 libcurl/7.88.1 (SecureTransport) LibreSSL/3.3.6 zlib/1.2.11 nghttp2/1.51.0
Features (libcurl):  alt-svc AsynchDNS HSTS HTTP2 IPv6 Largefile libz NTLM NTLM_WB SPNEGO SSL UnixSockets
Features (built-in): brotli
@RickMoynihan RickMoynihan added the bug Something isn't working label Oct 2, 2023
@jcamiel
Copy link
Collaborator

jcamiel commented Oct 2, 2023

Hi @RickMoynihan thanks for the issue, effectively we should also set request headers on each redirection, like curl. Reading the curl manual, it seems that authentication header (with --user) are not forwarded unless --location-trusted. We may add this option also

@RickMoynihan
Copy link
Author

Yeah, I'd seen that too and I think that would be great idea! 👍

I'm not sure what your approach to backwards compatability is, but it strikes me that changing the behaviour here would potentially break some hurl users.

@jcamiel
Copy link
Collaborator

jcamiel commented Oct 2, 2023

Yes good catch, we've already be bitten by compatibility issue in the past (mostly binary compatibility issues using hurl Rust crate), but also by fixing some bugs (for instance in how JSONPath queries are resolved). We need to see with the two others maintainers (@fabricereix and @lepapareil) how to deal with it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants