lib: deprecate _tls_common and _tls_wrap #57643

dario-piotrowicz · 2025-03-27T00:44:59Z

This PR is for runtime deprecation of the _tls_common and _tls_wrap modules

My hope is to manage to gradually deprecate the various _ modules that are not documented
and that should not be part of node.js' public API (for more context see: #57540 (comment))

Interestingly as you can see:

common and wrap expose respectively:

SecureContext
createSecureContext
translatePeerCertificate
and
TLSSocket
Server
createServer
connect

All of these, seem to already being re-exported by node:tls:

node/lib/tls.js

Lines 389 to 394 in 186bbf7

    
           exports.createSecureContext = _tls_common.createSecureContext; 
        
           exports.SecureContext = _tls_common.SecureContext; 
        
           exports.TLSSocket = _tls_wrap.TLSSocket; 
        
           exports.Server = _tls_wrap.Server; 
        
           exports.createServer = _tls_wrap.createServer; 
        
           exports.connect = _tls_wrap.connect;

the only exception being translatePeerCertificate (which I am not really very familiar with, but by the looks of it
seems like a rather simple function that can be re-implemented externally)

So I don't think that deprecating these modules should have huge impacts to users and that importing from
node:tls instead is a viable alternative

@BridgeAR 🙂👍

nodejs-github-bot · 2025-03-27T00:45:04Z

Review requested:

@nodejs/crypto
@nodejs/net

dario-piotrowicz · 2025-03-27T00:46:21Z

@BridgeAR I know you said that the runtime deprecation wasn't completely necessary but I really wanted to give it a shot (and I really like how it turned out 😄) I hope you don't mind 🙂

codecov · 2025-03-27T03:43:41Z

Codecov Report

Attention: Patch coverage is 94.57203% with 104 lines in your changes missing coverage. Please review.

Project coverage is 90.24%. Comparing base (62426a7) to head (15a47d9).
Report is 5 commits behind head on main.

Files with missing lines	Patch %	Lines
lib/internal/tls/wrap.js	94.32%	98 Missing and 2 partials ⚠️
lib/internal/tls/common.js	97.03%	4 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##             main   #57643   +/-   ##
=======================================
  Coverage   90.23%   90.24%           
=======================================
  Files         630      632    +2     
  Lines      185496   185475   -21     
  Branches    36367    36376    +9     
=======================================
- Hits       167388   167375   -13     
+ Misses      11000    10998    -2     
+ Partials     7108     7102    -6

Files with missing lines	Coverage Δ
lib/_tls_common.js	`100.00% <100.00%> (+2.56%)`	⬆️
lib/_tls_wrap.js	`100.00% <100.00%> (+5.61%)`	⬆️
lib/internal/crypto/x509.js	`91.75% <100.00%> (ø)`
lib/tls.js	`95.93% <100.00%> (ø)`
lib/internal/tls/common.js	`97.03% <97.03%> (ø)`
lib/internal/tls/wrap.js	`94.32% <94.32%> (ø)`

... and 26 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.

mcollina

lgtm

mcollina

lgtm

nodejs-github-bot · 2025-04-07T08:12:23Z

CI: https://ci.nodejs.org/job/node-test-pull-request/66100/

runtime deprecate the _tls_common and _tls_wrap modules, users should use nust node:tls insteal and internally internal/tls/commond and internal/tls/wrap should be used instead

restrict exports of _tls_common and _tls_wrap modules

nodejs-github-bot · 2025-04-12T18:07:48Z

CI: https://ci.nodejs.org/job/node-test-pull-request/66217/

jasnell · 2025-04-12T18:09:56Z

doc/api/deprecations.md

+Type: Runtime
+
+The `node:_tls_common` and `node:_tls_wrap` modules are deprecated as they should be considered
+an internal nodejs implementation rather than a public facing API, use `node:tls` instead.


Just noting... it's an unfortunate truth that while these are deprecated we likely will never be able to actually remove them. See require('sys') ....

oh I see.. that's pretty disappointing 🥲 (although I understand the security implications...)

I like your suggestion of throwing an error though, it'd be pretty nice if we could do that at some point 🙂

mh... or maybe the _ modules could be removed at some point since npm doesn't support such modules anyways? 🤔

#26292 (comment)
https://github.com/npm/validate-npm-package-name?tab=readme-ov-file#naming-rules

🤔

npm may not but an attacker could still locally inject a module which would still be problematic.

oh I see what you mean... like with a postinstall script or something that drops an _ module in the node_modules right? 🤔

oh too bad then 😓 (but I am still hoping for the throwing idea! 😀)

nodejs-github-bot added lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run. labels Mar 27, 2025

dario-piotrowicz force-pushed the dario/deprecate_tls branch 2 times, most recently from 6d4746b to 50c51bd Compare March 27, 2025 00:52

mcollina approved these changes Mar 27, 2025

View reviewed changes

lpinca added the needs-citgm PRs that need a CITGM CI run. label Mar 27, 2025

jasnell approved these changes Mar 27, 2025

View reviewed changes

marco-ippolito approved these changes Mar 28, 2025

View reviewed changes

dario-piotrowicz force-pushed the dario/deprecate_tls branch from 9d8afa9 to 0643430 Compare April 6, 2025 18:23

mcollina approved these changes Apr 7, 2025

View reviewed changes

mcollina added request-ci Add this label to start a Jenkins CI on a PR. semver-major PRs that contain breaking changes and should be released in the next major version. labels Apr 7, 2025

github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Apr 7, 2025

dario-piotrowicz force-pushed the dario/deprecate_tls branch from 0643430 to f09a338 Compare April 12, 2025 15:42

dario-piotrowicz added 2 commits April 12, 2025 18:18

lib: deprecate _tls_common and _tls_wrap

9b71272

runtime deprecate the _tls_common and _tls_wrap modules, users should use nust node:tls insteal and internally internal/tls/commond and internal/tls/wrap should be used instead

fixup! lib: deprecate _tls_common and _tls_wrap

15a47d9

restrict exports of _tls_common and _tls_wrap modules

dario-piotrowicz force-pushed the dario/deprecate_tls branch from f09a338 to 15a47d9 Compare April 12, 2025 17:18

jasnell reviewed Apr 12, 2025

View reviewed changes

jasnell approved these changes Apr 12, 2025

View reviewed changes

anonrig approved these changes Apr 12, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

lib: deprecate _tls_common and _tls_wrap #57643

lib: deprecate _tls_common and _tls_wrap #57643

dario-piotrowicz commented Mar 27, 2025 •

edited

Loading

nodejs-github-bot commented Mar 27, 2025

dario-piotrowicz commented Mar 27, 2025 •

edited

Loading

codecov bot commented Mar 27, 2025 •

edited

Loading

mcollina left a comment

mcollina left a comment

nodejs-github-bot commented Apr 7, 2025

nodejs-github-bot commented Apr 12, 2025

jasnell Apr 12, 2025

dario-piotrowicz Apr 12, 2025

dario-piotrowicz Apr 12, 2025 •

edited

Loading

jasnell Apr 12, 2025

dario-piotrowicz Apr 12, 2025

	exports.createSecureContext = _tls_common.createSecureContext;
	exports.SecureContext = _tls_common.SecureContext;
	exports.TLSSocket = _tls_wrap.TLSSocket;
	exports.Server = _tls_wrap.Server;
	exports.createServer = _tls_wrap.createServer;
	exports.connect = _tls_wrap.connect;

lib: deprecate _tls_common and _tls_wrap #57643

Are you sure you want to change the base?

lib: deprecate _tls_common and _tls_wrap #57643

Conversation

dario-piotrowicz commented Mar 27, 2025 • edited Loading

nodejs-github-bot commented Mar 27, 2025

dario-piotrowicz commented Mar 27, 2025 • edited Loading

codecov bot commented Mar 27, 2025 • edited Loading

Codecov Report

mcollina left a comment

Choose a reason for hiding this comment

mcollina left a comment

Choose a reason for hiding this comment

nodejs-github-bot commented Apr 7, 2025

nodejs-github-bot commented Apr 12, 2025

jasnell Apr 12, 2025

Choose a reason for hiding this comment

dario-piotrowicz Apr 12, 2025

Choose a reason for hiding this comment

dario-piotrowicz Apr 12, 2025 • edited Loading

Choose a reason for hiding this comment

jasnell Apr 12, 2025

Choose a reason for hiding this comment

dario-piotrowicz Apr 12, 2025

Choose a reason for hiding this comment

dario-piotrowicz commented Mar 27, 2025 •

edited

Loading

dario-piotrowicz commented Mar 27, 2025 •

edited

Loading

codecov bot commented Mar 27, 2025 •

edited

Loading

dario-piotrowicz Apr 12, 2025 •

edited

Loading