This repository was archived by the owner on Jan 23, 2023. It is now read-only.
Port HttpListener fixes to release/2.0.0 #20422
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
* Re-enable failing httplistener tests on osx. * Fix edge cases in httplistener prefixes. * Throw exception for all unresolved hostnames.
If the ctor encounters a failure, _processPrefixListener may remain null; then when the factory is Dispose'd, it null refs. This is taking out a bunch of runs, including almost all macOS runs.
Lack of doing so results in hangs if the IAsyncResult is from a different HttpListener
* Add HttpListenerResponse tests * Baseline all the managed test failures * Preliminary PR feedback - Remove added argument validation - Split up tests - Start to investigate fixing race conditions in the tests (this isn't done yet) * Baseline all HttpListener disabled tests on Unix * Use exact number of bytes to avoid race conditions Fixes failures when I stress test the suit by running the tests 100 times * Fix interminate test failures discovered by running the tests 100 times * Fix managed test failures and use overridable ConditionalFact/Theory instead of ActiveIssue This lets me simulate being on a managed environment on Windows so I can iterate more rapidly * Fix Windows 7 outerloop tests
If it doesn't receive the expected amount in the first iteration, it's likely it'll never break out of the loop. It's also not technically correct with regards to UTF8 handling, but that's unlikely to be an issue with our current tests.
Move the Windows implementation to be shared and remove the managed-specific implementation.
We shouldn't call CheckSentHeaders, as the Windows implementation doesn't. And once that's removed, it's the same as the Windows implementation and we can move the implementation to shared.
Move the Windows implementation to shared and use it from managed.
Move the Windows implementation to shared and use it from managed.
Move Windows implementation to shared, then use that from managed, which means moving the core of the managed implementation to a private EnsureResponseStream helper.
Move Windows implementation to shared, and rewrite managed implementation to use it. Remove setting of KeepAlive header, and fix up handling of content length with regards to chunked, Close(..., willBlock:false), etc.
* Add HttpListener authorization tests * Baseline managed test failures * Fix non-Windows test failures I don't know why these are failing but I don't think it's a bug as Windows and managed share the same code here
It only currently passes on Windows because the Windows implementation actually does some work, making it unlikely that the test will fail, but still possible. On Unix, the implementation is a stub and completes synchronously, thereby always failing.
Simply reuse the Windows implementation's cookie handling, moving it to a shared file, calling it from the managed, and removing the managed's implementation.
…res in CI I've been unable to reliably repro hangs on Unix locally. As a stand-in, I'm running the managed HttpListener implementation on Windows. There must be enough differences in timing or client behavior or Socket behavior or whatever to trigger a multitude of hangs/asserts, even though this should work reliably. Since it's likely that some subset of these tests are the same ones failing in CI, I've temporarily disabled them all, in hopes of getting CI green reliably. We should aggressively investigate and get these running again; the underlying cause(s) is likely something in the product that would impact real scenarios. A large chunk of them have to do with chunked encoding, so there's very likely a product issue there; also with establishing web socket connections.
…lure The managed implementation of HttpListener launches an asynchronous accept loop: each accept spawns the next async accept. If an async accept fails, for some reason the current code is actually closing the previously accepted connection, which doesn't make sense. Stop doing that.
- Cleanup some test code - Add HttpListener web socket tests - Fix some more Windows porting bugs - Baseline managed test failures
- Change CopyPrincipal to throw PNSE instead of NotImplementedException - Fix test attribution to allow "msbuild /t:rebuildandtest" to work with both Windows and managed implementations - Fix IsWindowsImplementation to recognize both netcoreapp and netfx implementations - Fix Assert.Equal calls that should be case-insensitive - Move Get_Disposed_ThrowsObjectDisposedException to also apply to managed implementation - Remove unnecessary Task disposal in HttpListenerWebSocketTests - Add missing await of ClientConnectTask in several tests, where the lack of it resulted in sporadic failures due to race conditions - Replace ReceiveAsync calls with custom ReceiveAllAsync calls where necessary
|
@dotnet-bot help |
|
Welcome to the dotnet/corefx Repository The following is a list of valid commands on this PR. To invoke a command, comment the indicated phrase on the PR The following commands are valid for all PRs and repositories. Click to expand
The following jobs are launched by default for each PR against dotnet/corefx:release/2.0.0. Click to expand
The following optional jobs are available in PRs against dotnet/corefx:release/2.0.0. Click to expand
Have a nice day! |
|
@dotnet-bot test outerloop netfx Windows_NT Debug |
|
LGTM |
|
@dotnet-bot test OuterLoop netfx Windows_NT Debug x64 please (https://github.com/dotnet/corefx/issues/11737) |
|
@dotnet-bot test OuterLoop netfx Windows_NT Debug x64 please |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
10 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This ports all changes to HttpListener since we forked the release/2.0.0 branch on May 17th. We can separately do a clean-up port to address any remaining issues addressed today/tomorrow.
Fixes #20060.
cc: @geoffkizer