-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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
Credential fixes #61114
Credential fixes #61114
Conversation
Tagging subscribers to this area: @dotnet/ncl Issue Detailsnull
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
Seems like we are hiding the password from the scanner .... but I think it is ok for the tests.
* fix HttpClientHandlerTest.RemoteServer.cs * fix HttpClientHandlerTest.Authentication.cs * fix HttpClientHandlerTest.cs
@@ -421,7 +421,7 @@ public async Task PostAsync_ManyDifferentRequestHeaders_SentCorrectly() | |||
request.Headers.Add("Access-Control-Request-Method", "GET"); | |||
request.Headers.Add("Access-Control-Request-Headers", "GET"); | |||
request.Headers.Add("Age", "12"); | |||
request.Headers.Authorization = new AuthenticationHeaderValue("Basic", "QWxhZGRpbjpvcGVuIHNlc2FtZQ=="); | |||
request.Headers.Authorization = new AuthenticationHeaderValue("Basic", authSafeValue); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just curious, how did extracting this into a variable help credscan?
@@ -203,7 +203,7 @@ public static IEnumerable<object[]> Authentication_TestData() | |||
{ | |||
yield return new object[] { "Basic realm=\"testrealm\"", true }; | |||
yield return new object[] { "Basic ", true }; | |||
yield return new object[] { "Basic realm=withoutquotes", true }; | |||
yield return new object[] { "Basic realm=PLACEHOLDERwithoutquotes", true }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
credscan picked up on the "realm" part here? but not on line 204? It is strange...
No description provided.