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

Use IndexOfAny in HttpListener.HandleAuthentication #71137

Merged
merged 2 commits into from
Jun 23, 2022

Conversation

stephentoub
Copy link
Member

Just cleaning up a manual loop that can instead be IndexOfAny.

@ghost
Copy link

ghost commented Jun 22, 2022

Tagging subscribers to this area: @dotnet/ncl
See info in area-owners.md if you want to be subscribed.

Issue Details

Just cleaning up a manual loop that can instead be IndexOfAny.

Author: stephentoub
Assignees: stephentoub
Labels:

area-System.Net.Http

Milestone: -

@wfurt
Copy link
Member

wfurt commented Jun 22, 2022

C:\h\w\A2D70905\w\A3BB094B\e>"C:\h\w\A2D70905\p\dotnet.exe" exec --runtimeconfig System.Net.HttpListener.Tests.runtimeconfig.json --depsfile System.Net.HttpListener.Tests.deps.json xunit.console.dll System.Net.HttpListener.Tests.dll -xml testResults.xml -nologo -nocolor -notrait category=IgnoreForCI -notrait category=OuterLoop -notrait category=failing  
  Discovering: System.Net.HttpListener.Tests (method display = ClassAndMethod, method display options = None)
  Discovered:  System.Net.HttpListener.Tests (found 286 of 299 test cases)
  Starting:    System.Net.HttpListener.Tests (parallel test collections = on, max threads = 2)
    System.Net.Tests.HttpListenerAuthenticationTests.NtlmAuthentication_InvalidRequestHeaders_ReturnsExpectedStatusCode(header: null, statusCode: Unauthorized) [FAIL]
      System.ArgumentOutOfRangeException : Length cannot be less than zero. (Parameter 'length')
      Stack Trace:
        /_/src/libraries/System.Private.CoreLib/src/System/String.Comparison.cs(385,0): at System.String.Compare(String strA, Int32 indexA, String strB, Int32 indexB, Int32 length, StringComparison comparisonType)
        /_/src/libraries/System.Net.HttpListener/src/System/Net/Windows/HttpListener.Windows.cs(816,0): at System.Net.HttpListener.HandleAuthentication(HttpListenerSession session, RequestContextBase memoryBlob, Boolean& 

looks related

Just cleaning up a manual loop that can instead be IndexOfAny.
{
break;
}
index = authorizationHeader.Length;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will work. But I think this is only used in the condition bellow to guard if any character was found. e.g. you can simplify that without assignment as

-  if (index < authorizationHeader.Length)
+  if (index >= 0)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

index is also referenced later in the method.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, I missed that reading it on GH. However, without going to the first block, headerScheme should remain AuthenticationSchemes.None and we will error out without reaching the next index use. Not sure if HttpListener is worth of any more troubles.

Copy link
Member

@wfurt wfurt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@stephentoub stephentoub merged commit 982946a into dotnet:main Jun 23, 2022
@stephentoub stephentoub deleted the listenerindexofany branch June 23, 2022 00:58
@karelz karelz added this to the 7.0.0 milestone Jul 19, 2022
@ghost ghost locked as resolved and limited conversation to collaborators Aug 18, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants