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

HttpRequestMessage gets reused multiple times which is not supported with the SocketHttpHandler #8661

Closed
campersau opened this issue Oct 4, 2019 · 5 comments · Fixed by NuGet/NuGet.Client#3078, NuGet/NuGet.Client#3122 or NuGet/NuGet.Client#3120

Comments

@campersau
Copy link

The SocketHttpHandler does not support reusing HttpRequestMessages multiple times. This can lead to issues with authentification if the user provides the wrong credentials the first time, because the Authorization header gets added by the SocketHttpHandler and is resend on the next try. In combination with some servers (e.g. the GitHub NuGet feed) this results in an loop where each time the same credentials are being passed in because this server does not send a Www-Authenticate header if an Authorization header is provided and so the new credentials are never added.

To prevent this issue it is currently possible to disable the SocketHttpHandler which I have done here NuGetPackageExplorer/NuGetPackageExplorer#841

See also https://github.com/dotnet/corefx/issues/25163#issuecomment-538009706

Sample Project

[Fact]
public async Task GitHubFeedTest()
{
    // uncomment this to fix the issue
    //AppContext.SetSwitch("System.Net.Http.UseSocketsHttpHandler", false);

    HttpHandlerResourceV3.CredentialService = new Lazy<ICredentialService>(() => new CredentialService(
        new AsyncLazy<IEnumerable<ICredentialProvider>>(
            () => Task.FromResult<IEnumerable<ICredentialProvider>>(new ICredentialProvider[] { new TestCredentialProvider()})
        ),
        nonInteractive: false,
        handlesDefaultCredentials: true
    ));
    // change `username` in this url to your user
    var source = Repository.Factory.GetCoreV3("https://nuget.pkg.github.com/username/index.json");
    var sourceProvider = await source.GetResourceAsync<PackageSearchResource>();

    await sourceProvider.SearchAsync("", new SearchFilter(false), 0, 10, NullLogger.Instance, CancellationToken.None);
}
private class TestCredentialProvider : ICredentialProvider
{
    public string Id => "test";
    public Task<CredentialResponse> GetAsync(Uri uri, IWebProxy proxy, CredentialRequestType type, string message, bool isRetry, bool nonInteractive, CancellationToken cancellationToken)
    {
        if (!isRetry)
        {
            return Task.FromResult(new CredentialResponse(new NetworkCredential("a", "b"))); // intentional wrong user and password
        }
        return Task.FromResult(new CredentialResponse(new NetworkCredential("username", "token"))); // change `username` and `token` to your github user and [token](https://github.com/settings/tokens) with package access
    }
}
@rrelyea
Copy link
Contributor

rrelyea commented Oct 4, 2019

@zivkan - related to some of the issues you are seeing?

@zivkan
Copy link
Member

zivkan commented Oct 4, 2019

no. SocketHttpHandler is the new HttpClient code in .NET Core 2.1. I've been testing exclusively on .NET Framework, which uses the WinHttpHandler.

@rrelyea
Copy link
Contributor

rrelyea commented Oct 7, 2019

@zivkan - assigning this issue to you, as it appears to affect GPR scenarios. See https://github.com/dotnet/corefx/issues/25163#issuecomment-538009706
FYI: @anangaur

@zivkan
Copy link
Member

zivkan commented Nov 13, 2019

We're reverting the first PR that closed this issue, due to the problems it introduced in .NET Core 3. The team's not comfortable with using and supporting reflection, so we'd like to investigate another long term fix, rather than patching the reflection issue.. Reopening this issue as it needs to be addressed again.

@zivkan zivkan reopened this Nov 13, 2019
@zivkan zivkan modified the milestones: 5.5, Backlog Nov 13, 2019
@clairernovotny
Copy link

This issue needs to be reopened now that the original fix was reverted.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment