Improve unmapped error exception to include the mapping function name and the error code#115485
Conversation
… and the error code
There was a problem hiding this comment.
Pull Request Overview
This PR enhances unmapped OpenSSL error handling by including the mapping function name and the raw error code in the thrown exception to aid diagnostics.
- Updated
MapOpenSsl*Codemethods to throw a detailed exception instead of a generic one. - Added a
GetUnmappedCodeExceptionhelper that formats the error message. - Introduced a new resource string for the unmapped OpenSSL code error.
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/OpenSslX509ChainProcessor.cs | Replaced generic CryptographicException throws with calls to GetUnmappedCodeException, and added that helper method. |
| src/libraries/System.Security.Cryptography/src/Resources/Strings.resx | Added Cryptography_UnmappedOpenSslCode resource string for formatting the new exception message. |
Comments suppressed due to low confidence (1)
src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/OpenSslX509ChainProcessor.cs:1423
- Consider adding unit tests to verify that when an unmapped OpenSSL error code is encountered, the exception message includes the correct function name and error code.
private static CryptographicException GetUnmappedCodeException(string functionName, int code)
|
Tagging subscribers to this area: @dotnet/area-system-security, @bartonjs, @vcsjones |
bartonjs
left a comment
There was a problem hiding this comment.
LGTM.
I sort of want to ask "should we get rid of the assert now that we have enough data?", but, eh, in a Debug build we should definitely be seeing these and mapping them... and it shows "this exception is not something we ever want someone to see". So... I like it as it is. ("Good talk.")
When we encounter an unmapped OpenSSL error, retail builds do not include any information about what the error was. This adds some exception text to help diagnose.
Contributes to #114129.