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

Custom Protocol not allowed in Location Header #3293

Closed
niklasweimann opened this issue Oct 8, 2024 · 4 comments · Fixed by #3382
Closed

Custom Protocol not allowed in Location Header #3293

niklasweimann opened this issue Oct 8, 2024 · 4 comments · Fixed by #3382
Assignees
Labels
bug Something isn't working
Milestone

Comments

@niklasweimann
Copy link

What is the current bug behavior?

When using a custom protocol e.g. "market://" to address the Google Play Store on a android device, hurl is complaining that the protocol should be http or https

Steps to reproduce

Call an endpoint that returns a HTTP 307 with Location: market://de.XXXX.XXXX.

What is the expected correct behavior?

Hurl should be able to assert on this Headers too

Execution context

  • Hurl Version (hurl --version):
    hurl 5.0.1 (x86_64-apple-darwin23.0) libcurl/8.7.1 (SecureTransport) LibreSSL/3.3.6 zlib/1.2.12 nghttp2/1.61.0
    Features (libcurl): alt-svc AsynchDNS HSTS HTTP2 IPv6 Largefile libz NTLM SPNEGO SSL UnixSockets
    Features (built-in): brotli
@niklasweimann niklasweimann added the bug Something isn't working label Oct 8, 2024
@fabricereix
Copy link
Collaborator

Thanks @niklasweimann for reporting the bug.
We initially added several checks to focus on http/https protocol.
But for sure, we should accept any Location header when we do not follow redirect.

@niklasweimann
Copy link
Author

I believe this limitation makes sense when the "follow redirects" feature is enabled. However, I wasn't aware of the "follow redirects" feature and initially thought the assertions were purely text-based comparisons. It might be helpful to add an additional assertion keyword like "raw equals" to handle cases like this. For example:

GET https://example.org
HTTP 302
[Asserts]
header "Location" raw equals "www.example.net"

In this case, "raw equals" would bypass all limitations and directly compare the string "www.example.net" with the value of the "Location“ header.

@fabricereix
Copy link
Collaborator

The comparisons are already text-based.
Like curl, following redirect is turned off by default and explicitly turned on with --location option.
The assert and the redirect should be independent of each other.

@jcamiel
Copy link
Collaborator

jcamiel commented Oct 17, 2024

See #3314, we should:

  • allow any value for header Location (could be file:///etc/passdw)
  • check at runtime that executed URL are only http:// , https:// (even during redirection)

Example:

GET {{host}}

Run with hurl --variable=file:///tmp/foo.txt

$ hurl --variable=file:///tmp/foo.txt
error: HTTP connection
  --> -:1:5
   |
 1 | GET {{host}}
   |     ^^^^^^^^ could not parse Response
   |

With Hurl 5.0.1, the curl transfer happens, we should prevent it.

@jcamiel jcamiel self-assigned this Oct 21, 2024
@jcamiel jcamiel added this to the 6.0.0 milestone Nov 8, 2024
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