Skip to content

Comments

Fix nondeterministic test failure for X509CertificateLoader#124617

Merged
vcsjones merged 3 commits intodotnet:mainfrom
vcsjones:fix-123630
Feb 19, 2026
Merged

Fix nondeterministic test failure for X509CertificateLoader#124617
vcsjones merged 3 commits intodotnet:mainfrom
vcsjones:fix-123630

Conversation

@vcsjones
Copy link
Member

OpenSSL does not block trailing data after an X.509 certificate like the test thought it did. OpenSSL permits trailing data as long as it is valid ASN.1. The unit test in question used the process ID as the trailing data. Certain PIDs resemble ASN.1 enough that it allows the parsing to succeed and not throw an exception like the test expected. For example, if the PID was 32816, this would be 30 80 00 00 - an indefinite length ASN.1 BER SEQUENCE.

The trailing data is ignored when the parsing succeeds, it's just a matter of if the trailing data causes an error (invalid ASN.1) or is ignored (valid ASN.1, but remains unconsumed).

Fixes #123630

@vcsjones vcsjones requested review from bartonjs and Copilot February 19, 2026 19:49
@vcsjones vcsjones added area-System.Security test-bug Problem in test source code (most likely) labels Feb 19, 2026
OpenSSL does not "block" trailing data after an X.509 certificate that the test thought it did. OpenSSL permits trailing data... as long as it is valid ASN.1.
Certain PIDs resemble ASN.1 enough that it allows the parsing to succeed and not throw an exception like the test expected. For example, if the PID was 32816,
this would be 30 80 00 00 - an indefinite length ASN.1 BER SEQUENCE.

The trailing data is ignored when the parsing succeeds, its just a matter of if the trailing data causes an error (invalid ASN.1) or is ignored (valid ASN.1, but remains unconsumed).
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 fixes a nondeterministic test failure in X509CertificateLoader tests. The test LoadWrappingCertificate_PEM_WithTrailingData was failing intermittently because it used the process ID as trailing data after a certificate. OpenSSL permits trailing data as long as it is valid ASN.1, and certain PIDs could accidentally resemble valid ASN.1 structures (e.g., PID 32816 = 30 80 00 00 = indefinite length ASN.1 BER SEQUENCE), causing the test to behave differently on each run depending on the PID value.

Changes:

  • Replaced nondeterministic PID-based trailing data with deterministic byte sequence [1, 2, 3, 4]
  • Removed trailing whitespace on line 157

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings February 19, 2026 19:53
@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.

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

Copilot reviewed 1 out of 1 changed files in this pull request and generated 2 comments.

@vcsjones vcsjones enabled auto-merge (squash) February 19, 2026 20:19
@vcsjones vcsjones merged commit 4eebedb into dotnet:main Feb 19, 2026
88 of 90 checks passed
@vcsjones vcsjones deleted the fix-123630 branch February 19, 2026 23:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-System.Security test-bug Problem in test source code (most likely)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Failing test: System.Security.Cryptography.X509Certificates.Tests.X509CertificateLoaderTests_FromByteArray.LoadWrappingCertificate_PEM_WithTrailingData

2 participants