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

OnRedirect does not work with custom HttpClient #831

Open
DanielHabenicht opened this issue Jul 17, 2024 · 3 comments
Open

OnRedirect does not work with custom HttpClient #831

DanielHabenicht opened this issue Jul 17, 2024 · 3 comments
Labels

Comments

@DanielHabenicht
Copy link

DanielHabenicht commented Jul 17, 2024

If one needs a custom HttpClient, e.g. for certificate issues during testing the redirect functionalities of Flurl do not work.

var handler = new HttpClientHandler()
{
    ServerCertificateCustomValidationCallback = HttpClientHandler.DangerousAcceptAnyServerCertificateValidator
};

// Works if the HttpClient is not given as argument
var client = new FlurlClient(new HttpClient(handler)).OnRedirect(
    call =>
    {
        if (call.Redirect.Url.Host == "customHost")
        {
            call.Redirect.Url.Host = "test.domain.internal";
        }

        call.Redirect.Follow = true;
    });
client.Settings.Redirects.Enabled = false;

using var session = new CookieSession(client);
var result = await session
    .Request("http://google.com/")
    .GetAsync();

// result.StatusCode is 200 instead of the expected 301, Enabled = false should not have redirected.

This is unexpected behaviour or not documented.
Furthermore the OnRedirect Callback never gets called, as well as .WithAutoRedirect(false)

@tmenier
Copy link
Owner

tmenier commented Jul 18, 2024

Flurl needs to disable redirects on the HttpClientHandler in order to implement its own redirects features, and that's impossible to do after the HttpClient has been created. So not a bug, but you're right that it could be documented better.

@DanielHabenicht
Copy link
Author

Alright, maybe apart from the documentation side, one could throw an error or warn if it is unsupported? (or sort it out via custom typing, so that these Extension Methods are not callable if configured with a custom HttpClient)

Another thing to note is, that this breaks functionality for a use case that needs to ignore SSL Certificates, but also needs the Redirect Features.

Should I make a PR to the https://github.com/tmenier/flurl-site/ Repository with remarks on incompatibility?

@tmenier
Copy link
Owner

tmenier commented Jul 22, 2024

one could throw an error or warn if it is unsupported?

Here's the problem: If you provide your own HttpClient whose inner message handler has AllowAutoRedirect set to false, then Flurl's redirect features will work fine and there's no reason to throw or warn. But Flurl has no way to detect that - the handler isn't exposed via any property of HttpClient.

This gotcha is the main reason that 4.0 provides new hooks to configure the inner handler, which is preferred over providing your own client whenever possible. I think it would be preferable in your case.

Feel free to submit a PR for the docs if you want. The link above (under "a few words of caution") warns against tampering with AllowAutoRedirect and UseCookies, but not specific to the scenario of providing your own HttpClient.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants