-
Notifications
You must be signed in to change notification settings - Fork 10k
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
Harden IPv6Any error handling #45529
Conversation
cc @amcasey |
@JamesNK ? |
// HttpsConnectionMiddleware.CreateHttp3Options, Http3 doesn't support OnAuthenticate. | ||
&& ex is not NotSupportedException) |
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.
This isn't a strong guarantee, i.e. someone could change the exception thrown without realizing they change behavior here.
Is there test coverage for this?
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.
Well, there's this test 😁
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.
Is this the behavior we want for all NotSupportedException
s? If not, would it make more sense to throw a subtype?
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.
We avoid defining custom exception types without an explicit need. We can reconsider if we find a conflict. We could compare the messages if we're paranoid.
Co-authored-by: James Newton-King <james@newtonking.com>
Fixes flaky test #43548
This test starts the server and expect a NotSupportedException due to some TLS options we don't support with HTTP/3(Quic).
Buggy flow:
Fix: Don't fall back to IPv4Any for a NotSupportedException.
I also cleaned up some log statements and adjusted the port scanning to skip around more and avoid conflicts with other tests.