Do not allow forwarding of authorization headers on redirect.

[makrsmark]

There is now a flag auth_on_redirect that can be set if you need to pass authorization headers. This is similar to
https://curl.se/docs/CVE-2018-1000007.html
and
https://nvd.nist.gov/vuln/detail/CVE-2021-31879

[pcriv]

Having this same issue with redirects and the HTTPrb client, maybe you can update the PR to include that client?

[makrsmark]

which backend? from what i can tell the net_http backend is the only one that implements redirects as part of this library. I would assume that's a bug/issue for the client library

[janko]

Thanks for the pull request. It seems that this will apply only on Down::NetHttp#download but not Down::NetHttp#open. Could we add support for the latter as well? I don't remember now why these redirects implementations don't seem to share any common code.

[makrsmark]

Sure, I'll take a crack at it

[pcriv]

@janko does the fix for HTTPrb and HTTPX need to be done here or on their respective repos?

[makrsmark]

do i need this? or asked another way, should credentials stay when it's a relative redirect?

[makrsmark]

going to remove if it's not a relative redirect

[makrsmark]

same question in a different place - should there be some logic to keep the creds if it's a relative redirect?

[makrsmark]

there's not a good way to preserve on a relative redirect - i can check to see if the host is the same, but that doesn't help from a test perspective. for now, i'm keping the logic as-is.

[makrsmark]

@janko would you mind taking another look at this pr?

[HoneyryderChuck]

Fwiw httpx does this already (do try with a recent version).

[janko]

Looks good, thanks! Sorry it took forever to get merged 😅

[janko]

@makrsmark Hmm, there seem to be syntax errors even on newer Rubies. I don't mind supporting only Ruby 3.0+ starting from next release, so no need to worry about compatibility with older rubies.

[janko]

I see it's actually passing on Ruby 3.1+. All good then 🙂

[makrsmark]

@janko -syntax can be reverted to support older rubies, that was on me. I see some test failures that seem unrelated to my changes

  1) Failure:
Down::Httpx::#open#test_0011_closes the response body when content has been read [test/httpx_test.rb:260]:
not all expectations were satisfied
unsatisfied expectations:
- expected exactly once, invoked never: #<AnyInstance:HTTPX::Response::Body>.close(any_parameters)


  2) Error:
Down::Httpx::#open#test_0002_follows redirects:
Down::TooManyRedirects: too many redirects
    lib/down/httpx.rb:143:in `response_error!'
    lib/down/httpx.rb:116:in `request'
    lib/down/httpx.rb:88:in `open'
    lib/down/backend.rb:17:in `open'
    test/httpx_test.rb:190:in `block (3 levels) in <top (required)>'

  3) Failure:
Down::Httpx::#open#test_0014_raises on HTTP error responses [test/httpx_test.rb:280]:
Expected #<#<Class:0x0000000cb9419680>:2340> to be an instance of HTTPX::StreamResponse, not #<Class:0x0000000cb9419680>.

  4) Failure:
Down::Httpx::#open#test_0017_re-raises timeout errors [test/httpx_test.rb:304]:
Down::TimeoutError expected but nothing was raised.

  5) Failure:
Down::Httpx::#open#test_0012_closes the body on IO close [test/httpx_test.rb:266]:
not all expectations were satisfied
unsatisfied expectations:
- expected exactly once, invoked never: #<AnyInstance:HTTPX::Response::Body>.close(any_parameters)


  6) Failure:
Down::Httpx::#open#test_0006_saves response data [test/httpx_test.rb:235]:
Expected #<#<Class:0x0000000cb95ef9f0>:2360> to be an instance of HTTPX::StreamResponse, not #<Class:0x0000000cb95ef9f0>.

260 runs, 538 assertions, 5 failures, 1 errors, 0 skips

trying to fix here - #102