-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
[QUIC] Validate Close/Abort errorCode parameter #102264
Conversation
…ethods for better reuse
Tagging subscribers to this area: @dotnet/ncl |
/azp run runtime-libraries stress-http |
Azure Pipelines successfully started running 1 pipeline(s). |
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, one question.
@@ -447,6 +447,8 @@ public void Abort(QuicAbortDirection abortDirection, long errorCode) | |||
return; | |||
} | |||
|
|||
ThrowHelper.ValidateErrorCode(nameof(Abort), errorCode); |
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.
what about the compiler-filled parameter propertyName
, won't it produce weird error message? Same for QuicConnection.CloseAsync
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.
Yeah, you're right: System.ArgumentOutOfRangeException: 'errorCode' should be within [0, 4611686018427387903) range. (Parameter 'CloseAsync')
Changed to: System.ArgumentOutOfRangeException: 'CloseAsync.errorCode' should be within [0, 4611686018427387903) range. (Parameter 'errorCode')
obviously for both CloseAsync
and Abort
.
13ec061
to
c432881
Compare
* Validate Close/Abort errorCode parameter, shuffle around validation methods for better reuse * Remove unnecassary PNSE exclusion * Changed exception text and added tests.
Also shuffle around validation methods for better reuse and fix typo.
Fixes #101385