SocketsHttpHandler: fix logic to check for Proxy-support header#27886
SocketsHttpHandler: fix logic to check for Proxy-support header#27886geoffkizer merged 2 commits intodotnet:masterfrom
Conversation
|
|
||
| if (TryGetAuthenticationChallenge(response, isProxyAuth, authUri, credentials, out AuthenticationChallenge challenge) && | ||
| (!isProxyAuth || CheckIfProxySupportsConnectionAuth(response))) | ||
| if (!isProxyAuth && connection.UsingProxy && !CheckIfProxySupportsConnectionAuth(response)) |
There was a problem hiding this comment.
What does "IsProxyAuth" mean here?
There was a problem hiding this comment.
Could CheckIfProxySupportsConnectionAuth just be ProxySupportsConnectionAuth?
There was a problem hiding this comment.
Yeah, that's better, I'll change.
There was a problem hiding this comment.
Re isProxyAuth:
This routine (and the similar one in AuthenticationHelper for basic/digest) is used in two cases:
(1) Authenticate against proxy (isProxyAuth = true)
(2) Authenticate against origin server (isProxyAuth = false)
The logic is basically the same between these two cases, just different status code (407 vs 401) and different header names (Proxy-Authenticate vs Www-Authenticate, Proxy-Authorization vs Authorization). So we just use the isProxyAuth flag to distinguish.
For example, this helper is called:
private static HttpHeaderValueCollection<AuthenticationHeaderValue> GetResponseAuthenticationHeaderValues(HttpResponseMessage response, bool isProxyAuth)
{
return isProxyAuth ?
response.Headers.ProxyAuthenticate :
response.Headers.WwwAuthenticate;
}I'm open to a better parameter name than "isProxyAuth" if you have suggestions...
There was a problem hiding this comment.
Thanks for the explanation. Maybe just a comment on the parameter explaining what it is.
|
@wfurt, I'm going to merge. Let me know when you have a chance to try this in your setup. |
SocketsHttpHandler: fix logic to check for Proxy-support header Commit migrated from dotnet/corefx@ae60f0f
4 participants
Currently we're looking for this header when we do proxy auth. This is wrong. We should look for this header when we are trying to do regular auth through a proxy.
@davidsh @wfurt @stephentoub
Fixes #27872
@wfurt can you test and confirm that this fixes #27872? (Windows only)