Skip to content

tls: add unsupported renegotiation error#63161

Open
panva wants to merge 2 commits intonodejs:mainfrom
panva:even-more-boring
Open

tls: add unsupported renegotiation error#63161
panva wants to merge 2 commits intonodejs:mainfrom
panva:even-more-boring

Conversation

@panva
Copy link
Copy Markdown
Member

@panva panva commented May 6, 2026

tls: add unsupported renegotiation error

Map BoringSSL's native renegotiation failure to ERR_TLS_RENEGOTIATION_UNSUPPORTED when TLSSocket#renegotiate() is called. This avoids exposing a rather ugly ERR_SSL_FUNCTION_SHOULD_NOT_HAVE_BEEN_CALLED.


test: update tls/crypto behaviour expectations when using BoringSSL

I've been testing the BoringSSL coverage in #63125, as I slowly drain that stack we'll be able to add BoringSSL to the test-shared.yml workflow such that embedders do not have to disable tests or float patches.

panva added 2 commits May 6, 2026 21:15
Map BoringSSL's native renegotiation failure to
ERR_TLS_RENEGOTIATION_UNSUPPORTED when TLSSocket#renegotiate() is
called. This avoids exposing an implementation-specific OpenSSL error
when the TLS backend does not support caller-initiated renegotiation.

Signed-off-by: Filip Skokan <panva.ip@gmail.com>
Signed-off-by: Filip Skokan <panva.ip@gmail.com>
@panva panva added the commit-queue-rebase Add this label to allow the Commit Queue to land a PR in several commits. label May 6, 2026
@nodejs-github-bot
Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/crypto
  • @nodejs/net

@nodejs-github-bot nodejs-github-bot added errors Issues and PRs related to JavaScript errors originated in Node.js core. needs-ci PRs that need a full CI run. tls Issues and PRs related to the tls subsystem. labels May 6, 2026
@panva panva requested review from codebytere and jasnell May 6, 2026 19:24
@codecov
Copy link
Copy Markdown

codecov Bot commented May 6, 2026

Codecov Report

❌ Patch coverage is 77.77778% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.02%. Comparing base (3c2d2e3) to head (a536db8).
⚠️ Report is 5 commits behind head on main.

Files with missing lines Patch % Lines
lib/internal/tls/wrap.js 71.42% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #63161      +/-   ##
==========================================
- Coverage   90.04%   90.02%   -0.02%     
==========================================
  Files         713      713              
  Lines      224484   224505      +21     
  Branches    42430    42436       +6     
==========================================
- Hits       202134   202110      -24     
- Misses      14156    14224      +68     
+ Partials     8194     8171      -23     
Files with missing lines Coverage Δ
lib/internal/errors.js 97.65% <100.00%> (+<0.01%) ⬆️
lib/internal/tls/wrap.js 95.03% <71.42%> (-0.10%) ⬇️

... and 48 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@pimterry pimterry added the request-ci Add this label to start a Jenkins CI on a PR. label May 7, 2026
@panva panva added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label May 7, 2026
@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label May 7, 2026
@nodejs-github-bot
Copy link
Copy Markdown
Collaborator

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

Labels

author ready PRs that have at least one approval, no pending requests for changes, and a CI started. commit-queue-rebase Add this label to allow the Commit Queue to land a PR in several commits. errors Issues and PRs related to JavaScript errors originated in Node.js core. needs-ci PRs that need a full CI run. tls Issues and PRs related to the tls subsystem.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants