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.68846% with 104 lines in your changes missing coverage. Please review.

Project coverage is 90.18%. Comparing base (535c2f7) to head (f96445c).
Report is 25 commits behind head on main.

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

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #57643      +/-   ##
==========================================
+ Coverage   90.12%   90.18%   +0.06%     
==========================================
  Files         629      631       +2     
  Lines      186638   186662      +24     
  Branches    36618    36656      +38     
==========================================
+ Hits       168202   168344     +142     
+ Misses      11222    11122     -100     
+ Partials     7214     7196      -18

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%> (ø)`
src/node_builtins.cc	`79.17% <ø> (ø)`
lib/internal/tls/common.js	`97.43% <97.43%> (ø)`
lib/internal/tls/wrap.js	`94.38% <94.38%> (ø)`

... and 42 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/

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 · 2025-05-05T09:53:56Z

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

nodejs-github-bot · 2025-05-06T10:38:32Z

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

nodejs-github-bot · 2025-05-07T08:27:02Z

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

nodejs-github-bot · 2025-05-07T13:10:49Z

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

nodejs-github-bot · 2025-05-07T13:53:46Z

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

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

skip tests if crypo is missing

nodejs-github-bot · 2025-05-07T23:23:05Z

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

nodejs-github-bot · 2025-05-08T00:21:37Z

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

jasnell · 2025-05-09T18:49:11Z

lib/_tls_common.js

-// NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM,
-// DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
-// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE
-// USE OR OTHER DEALINGS IN THE SOFTWARE.


Best to leave the trademark headers in place even if the rest is being removed

ok thanks 🙂

or should I move those to the new internal modules which contain the original copy-pasted code?

Copyright comments restored (into the new files): f96445c

I hope this works for you 🙏

nodejs-github-bot · 2025-05-10T09:50:31Z

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

nodejs-github-bot · 2025-05-10T16:56:46Z

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

nodejs-github-bot · 2025-05-10T23:10:44Z

Landed in a822a1c

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 2 times, most recently 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

dario-piotrowicz added the request-ci Add this label to start a Jenkins CI on a PR. label May 5, 2025

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

dario-piotrowicz force-pushed the dario/deprecate_tls branch from 15a47d9 to 2ef23ad Compare May 6, 2025 17:10

dario-piotrowicz added the request-ci Add this label to start a Jenkins CI on a PR. label May 6, 2025

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

dario-piotrowicz added 3 commits May 7, 2025 23:54

lib: deprecate _tls_common and _tls_wrap

d366a14

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

0c3bb4c

skip tests if crypo is missing

add new internal modules to node_builtins.cc

49fd777

dario-piotrowicz force-pushed the dario/deprecate_tls branch from d03c289 to 49fd777 Compare May 7, 2025 22:55

anonrig approved these changes May 7, 2025

View reviewed changes

anonrig added commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. request-ci Add this label to start a Jenkins CI on a PR. labels May 7, 2025

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

BridgeAR approved these changes May 7, 2025

View reviewed changes

jasnell approved these changes May 9, 2025

View reviewed changes

restore copyright comments

f96445c

anonrig approved these changes May 9, 2025

View reviewed changes

dario-piotrowicz added the request-ci Add this label to start a Jenkins CI on a PR. label May 10, 2025

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

dario-piotrowicz added the commit-queue Add this label to land a pull request using GitHub Actions. label May 10, 2025

nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label May 10, 2025

nodejs-github-bot merged commit a822a1c into nodejs:main May 10, 2025
60 checks passed

dario-piotrowicz deleted the dario/deprecate_tls branch May 10, 2025 23:12

dario-piotrowicz mentioned this pull request May 14, 2025

lib: deprecate _stream_* modules #58337

Merged

juanarbol mentioned this pull request May 30, 2025

Getting TypeError: info.replace is not a function when using https.request #58509

Closed

This was referenced May 31, 2025

http2: deprecate priority #58313

Merged

[v24.x] Backport doc deprecations #58542

Closed

	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;

Uh oh!

lib: deprecate _tls_common and _tls_wrap #57643

lib: deprecate _tls_common and _tls_wrap #57643

Uh oh!

Conversation

dario-piotrowicz commented Mar 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nodejs-github-bot commented Mar 27, 2025

Uh oh!

dario-piotrowicz commented Mar 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented Mar 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

mcollina left a comment

Choose a reason for hiding this comment

Uh oh!

mcollina left a comment

Choose a reason for hiding this comment

Uh oh!

nodejs-github-bot commented Apr 7, 2025

Uh oh!

nodejs-github-bot commented Apr 12, 2025

Uh oh!

jasnell Apr 12, 2025

Choose a reason for hiding this comment

Uh oh!

dario-piotrowicz Apr 12, 2025

Choose a reason for hiding this comment

Uh oh!

dario-piotrowicz Apr 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jasnell Apr 12, 2025

Choose a reason for hiding this comment

Uh oh!

dario-piotrowicz Apr 12, 2025

Choose a reason for hiding this comment

Uh oh!

nodejs-github-bot commented May 5, 2025

Uh oh!

nodejs-github-bot commented May 6, 2025

Uh oh!

nodejs-github-bot commented May 7, 2025

Uh oh!

nodejs-github-bot commented May 7, 2025

Uh oh!

nodejs-github-bot commented May 7, 2025

Uh oh!

nodejs-github-bot commented May 7, 2025

Uh oh!

nodejs-github-bot commented May 8, 2025

Uh oh!

jasnell May 9, 2025

Choose a reason for hiding this comment

Uh oh!

dario-piotrowicz May 9, 2025

Choose a reason for hiding this comment

Uh oh!

dario-piotrowicz May 9, 2025

Choose a reason for hiding this comment

Uh oh!

nodejs-github-bot commented May 10, 2025

Uh oh!

nodejs-github-bot commented May 10, 2025

Uh oh!

Uh oh!

nodejs-github-bot commented May 10, 2025

Uh oh!

Uh oh!

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