Disallow automatic request redirection if the original request contains potentially sensitive headers

[ok2c]

@arturobernalg Please review and double-check.

[arturobernalg]

@ok2c LGTM

[dani0600]

Thanks for your work on the latest changes, @ok2c & @arturobernalg ! I've noticed you've implemented a new check to prevent redirects when requests contain sensitive headers, and also introduced a new LaxRedirectStrategy to allow more permissive redirects, this in v5.4.

I have a question regarding this code fragment: since LaxRedirectStrategy extends DefaultRedirectStrategy, and doesn't override the method isRedirectAllowed() containing the sensitive header check, wouldn't this mean that using LaxRedirectStrategy in a client would still fall back to the DefaultRedirectStrategy implementation of that method in the RedirectExec.handleResponse() call?

In that case, the check would still apply, and redirects would be blocked—even when using the lax strategy. Is this the intended behavior, or could this be an oversight?

Thanks in advance for clarifying!

PS: Unless I'm missing some configuration, this is happening in some of my tests, I could help you investigate. (Using CloseableHttpClient setting LaxRedirectStrategy into my client)

[ok2c]

could this be an oversight?

@dani0600 Truth to be told that was an oversight on my part. However, it may not be a bad thing that one must explicitly enable redirects with sensitive headers even when LaxRedirectStrategy is being used.

[dani0600]

I understand @ok2c, thanks for your clarifications :)

I've opened a Jira issue here: https://issues.apache.org/jira/browse/HTTPCLIENT-2383.

As I outlined in the ticket, I believe this behavior might lead to some unexpected outcomes and adds a bit of complexity when using HttpClient. I will prepare a PR to show a possible fix — feel free to take a look when you have a moment. I'd really appreciate your feedback on it.

Thanks again !