Skip to content

Replace AsnValueReader with ValueAsnReader and remove internal type#125290

Open
Copilot wants to merge 6 commits intomainfrom
copilot/replace-asnvaluer-reader-remove-asnvaluereader
Open

Replace AsnValueReader with ValueAsnReader and remove internal type#125290
Copilot wants to merge 6 commits intomainfrom
copilot/replace-asnvaluer-reader-remove-asnvaluereader

Conversation

Copy link
Contributor

Copilot AI commented Mar 7, 2026

Summary

Replaces all uses of the internal AsnValueReader type with the new public ValueAsnReader from System.Formats.Asn1 (introduced in #125254), then removes the internal type entirely.

Changes

Type replacement

  • NegotiateAuthenticationPal.ManagedSpnego.cs — replaced all AsnValueReader usages with ValueAsnReader (6 variable declarations)
  • CompositeMLDsaTestHelpers.cs — replaced AsnValueReader usages with ValueAsnReader (2 variable declarations)

File cleanup

  • Deleted AsnValueReader.cs — the internal AsnValueReader ref struct is no longer needed
  • Created AsnWriterExtensions.cs — extracted the AsnWriterExtensions class (which shared the file with AsnValueReader) into its own file under a new AsnWriter directory
  • Created System.Security.Cryptography.AsnWriter.Shared.projitems — new shared projitems file in the AsnWriter directory referencing AsnWriterExtensions.cs
  • Removed the old Asn1Reader directory entirely (deleted the old System.Security.Cryptography.Asn1Reader.Shared.projitems)
  • Updated all 5 csproj files that imported the old projitems to reference the new AsnWriter path: System.Security.Cryptography.csproj, System.Security.Cryptography.Tests.csproj, System.Security.Cryptography.Pkcs.csproj, Microsoft.Bcl.Cryptography.csproj, Microsoft.Bcl.Cryptography.Tests.csproj
  • Removed the projitems import from System.Net.Security.csproj — this project only used the shared items for AsnValueReader; it doesn't use AsnWriterExtensions

Validation

  • All affected projects build successfully: System.Net.Security, System.Security.Cryptography, System.Security.Cryptography.Pkcs, Microsoft.Bcl.Cryptography, and their test projects
  • Crypto test suite passes (4 pre-existing X509 chain/network failures unrelated to this change)
  • No remaining references to AsnValueReader in the codebase

✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

Copilot AI and others added 3 commits March 7, 2026 13:57
- Replace AsnValueReader with ValueAsnReader in NegotiateAuthenticationPal.ManagedSpnego.cs
- Replace AsnValueReader with ValueAsnReader in CompositeMLDsaTestHelpers.cs
- Extract AsnWriterExtensions to its own file
- Delete AsnValueReader.cs
- Update shared projitems to reference AsnWriterExtensions.cs
- Remove projitems import from System.Net.Security.csproj

Co-authored-by: vcsjones <361677+vcsjones@users.noreply.github.com>
Co-authored-by: vcsjones <361677+vcsjones@users.noreply.github.com>
@dotnet-policy-service
Copy link
Contributor

Tagging subscribers to this area: @bartonjs, @vcsjones, @dotnet/area-system-security
See info in area-owners.md if you want to be subscribed.

@vcsjones vcsjones marked this pull request as ready for review March 7, 2026 16:50
Copilot AI review requested due to automatic review settings March 7, 2026 16:50
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR completes the migration away from the internal AsnValueReader by switching remaining call sites to the public ValueAsnReader in System.Formats.Asn1, and then removing the internal implementation from the shared Asn1Reader items.

Changes:

  • Replaced remaining AsnValueReader usages with ValueAsnReader in SPNEGO parsing and CompositeMLDsa test helpers.
  • Removed the internal AsnValueReader type and split out the still-needed AsnWriterExtensions into its own file.
  • Updated build/project wiring to stop importing the shared Asn1Reader items into System.Net.Security and to reference the new AsnWriterExtensions.cs from the shared projitems.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.

Show a summary per file
File Description
src/libraries/System.Net.Security/src/System/Net/NegotiateAuthenticationPal.ManagedSpnego.cs Updates SPNEGO ASN.1 parsing to use ValueAsnReader instead of the removed internal reader.
src/libraries/System.Net.Security/src/System.Net.Security.csproj Removes the shared Asn1Reader projitems import now that AsnValueReader is no longer used by this project.
src/libraries/Common/tests/System/Security/Cryptography/AlgorithmImplementations/CompositeMLDsa/CompositeMLDsaTestHelpers.cs Updates test helper ASN.1 parsing to use ValueAsnReader.
src/libraries/Common/src/System/Security/Cryptography/Asn1Reader/System.Security.Cryptography.Asn1Reader.Shared.projitems Swaps shared compile item from AsnValueReader.cs to AsnWriterExtensions.cs.
src/libraries/Common/src/System/Security/Cryptography/Asn1Reader/AsnWriterExtensions.cs New standalone file containing the crypto-focused AsnWriter extension helpers.
src/libraries/Common/src/System/Security/Cryptography/Asn1Reader/AsnValueReader.cs Deletes the internal AsnValueReader implementation now that usages have been migrated.

You can also share your feedback on Copilot code review. Take the survey.

@vcsjones
Copy link
Member

vcsjones commented Mar 9, 2026

@copilot having AsnWriterExtensions.cs in a directory called Asn1Reader is a little weird now. Can you change directory to AsnWriter and update the project files?

Moved the file and projitems to a new AsnWriter directory since having
writer extensions in an Asn1Reader directory was confusing. Updated all
5 csproj imports to reference the new location.

Co-authored-by: vcsjones <361677+vcsjones@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Mar 9, 2026

@copilot having AsnWriterExtensions.cs in a directory called Asn1Reader is a little weird now. Can you change directory to AsnWriter and update the project files?

Done in 568b8d9. Moved AsnWriterExtensions.cs and the shared projitems from Asn1Reader to a new AsnWriter directory, and updated all 5 csproj imports to reference the new path. The old Asn1Reader directory is now fully removed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants