Use encode callback instead of renting a buffer to write to in DSAKeyFormatHelper#112638
Merged
vcsjones merged 2 commits intodotnet:mainfrom Feb 19, 2025
Merged
Use encode callback instead of renting a buffer to write to in DSAKeyFormatHelper#112638vcsjones merged 2 commits intodotnet:mainfrom
vcsjones merged 2 commits intodotnet:mainfrom
Conversation
Contributor
There was a problem hiding this comment.
PR Overview
This pull request updates the DSAKeyFormatHelper to use the new callback-based Encode API on AsnWriter instead of renting a buffer and copying data.
- Replaces direct buffer renting and TryEncode with a callback that writes a BitString directly to the writer
- Removes obsolete error handling via Debug.Fail and exception throwing on buffer encoding failure
Changes
| File | Description |
|---|---|
| src/libraries/Common/src/System/Security/Cryptography/DSAKeyFormatHelper.cs | Converted buffer-based encoding to use AsnWriter's callback-based Encode, eliminating CryptoPool usage |
Copilot reviewed 1 out of 1 changed files in this pull request and generated no comments.
Comments suppressed due to low confidence (1)
src/libraries/Common/src/System/Security/Cryptography/DSAKeyFormatHelper.cs:277
- Ensure that the updated callback properly handles encoding failures. In the previous code, an explicit Debug.Fail and exception were triggered when TryEncode failed; please verify that inner.Encode will throw an exception or otherwise signal failure if encoding does not succeed.
inner.Encode(writer, static (writer, encoded) => {
Tip: Copilot only keeps its highest confidence comments to reduce noise and keep you focused. Learn more
Contributor
|
Tagging subscribers to this area: @dotnet/area-system-security, @bartonjs, @vcsjones |
This was referenced Feb 18, 2025
Open
Closed
Closed
bartonjs
approved these changes
Feb 18, 2025
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
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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 is a place I missed when adopting the new callback-based
EncodeonAsnWriterin #106728. This avoids a rent and a copy of data.