Skip to content

fix X509Certificate::parse() throwing internal error for invalid input#6313

Merged
anonrig merged 1 commit intomainfrom
yagiz/fix-x509-certificate
Mar 13, 2026
Merged

fix X509Certificate::parse() throwing internal error for invalid input#6313
anonrig merged 1 commit intomainfrom
yagiz/fix-x509-certificate

Conversation

@anonrig
Copy link
Member

@anonrig anonrig commented Mar 12, 2026

Fixes a sentry issue

@anonrig anonrig requested a review from jasnell March 12, 2026 19:04
@anonrig anonrig requested review from a team as code owners March 12, 2026 19:04
@ask-bonk
Copy link
Contributor

ask-bonk bot commented Mar 12, 2026

OK, inspect will truncate large values, so that's fine. This is a clean, well-scoped fix. The change is:

  1. C++: Replace throwOpensslError with return kj::none when invalid cert data is passed
  2. TypeScript: Check for null return and throw a user-friendly ERR_INVALID_ARG_VALUE
  3. Type declaration: Updated to reflect X509Certificate | null return type
  4. Tests: Good coverage of several invalid input scenarios

The only potential concern is the Node.js compatibility difference in error codes, but this is actually an improvement since the prior behavior was throwing an internal error (which is worse). The PR description says it fixes a Sentry issue, which confirms this was crashing in production.

LGTM

github run

@anonrig anonrig enabled auto-merge March 12, 2026 19:11
@anonrig anonrig force-pushed the yagiz/fix-x509-certificate branch from 5b6057c to 1b5ccb1 Compare March 12, 2026 22:58
@anonrig anonrig merged commit 15392d4 into main Mar 13, 2026
22 of 23 checks passed
@anonrig anonrig deleted the yagiz/fix-x509-certificate branch March 13, 2026 00:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants