Skip to content

tls: use validateFunction for options.SNICallback - #50530

Merged
nodejs-github-bot merged 1 commit into
nodejs:mainfrom
deokjinkim:231103_tls_snicallback
Nov 10, 2023
Merged

tls: use validateFunction for options.SNICallback#50530
nodejs-github-bot merged 1 commit into
nodejs:mainfrom
deokjinkim:231103_tls_snicallback

Conversation

@deokjinkim

Copy link
Copy Markdown
Contributor

If user uses invalid type for options.SNICallback in TLSSocket(), it's not internal issue of Node.js. So validateFunction() is more proper than assert().

Below is shown if we run newly added test case without this PR.

=== release test-tls-snicallback-error === Path: parallel/test-tls-snicallback-error
node:assert:635
throw err;
^
AssertionError [ERR_ASSERTION]: Expected values to be strictly deep-equal:
+ actual - expected
Comparison {
+ code: 'ERR_INTERNAL_ASSERTION',
+ name: 'Error'
- code: 'ERR_INVALID_ARG_TYPE',
- name: 'TypeError'
}
at Object.<anonymous> (/home/deokjinkim/oss/node2/test/parallel/test-tls-snicallback-error.js:18:10)
at Module._compile (node:internal/modules/cjs/loader:1376:14)
at Module._extensions..js (node:internal/modules/cjs/loader:1435:10)
at Module.load (node:internal/modules/cjs/loader:1207:32)
at Module._load (node:internal/modules/cjs/loader:1023:12)
at Function.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:135:12)
at node:internal/main/run_main_module:28:49 {
generatedMessage: true,
code: 'ERR_ASSERTION',
actual: Error [ERR_INTERNAL_ASSERTION]: This is caused by either a bug in Node.js or incorrect usage of Node.js internals.
Please open an issue with this stack trace at https://github.com/nodejs/node/issues
at assert (node:internal/assert:14:11)
at TLSSocket._init (node:_tls_wrap:912:5)
at new TLSSocket (node:_tls_wrap:591:8)
at assert.throws.code (/home/deokjinkim/oss/node2/test/parallel/test-tls-snicallback-error.js:19:5)
at getActual (node:assert:756:5)
at Function.throws (node:assert:902:24)
at Object.<anonymous> (/home/deokjinkim/oss/node2/test/parallel/test-tls-snicallback-error.js:18:10)
at Module._compile (node:internal/modules/cjs/loader:1376:14)
at Module._extensions..js (node:internal/modules/cjs/loader:1435:10)
at Module.load (node:internal/modules/cjs/loader:1207:32) {
code: 'ERR_INTERNAL_ASSERTION'
},
expected: { code: 'ERR_INVALID_ARG_TYPE', name: 'TypeError' },
operator: 'throws'
}
Node.js v22.0.0-pre
Command: out/Release/node /home/deokjinkim/oss/node2/test/parallel/test-tls-snicallback-error.js

@nodejs-github-botnodejs-github-bot added needs-ci PRs that need a full CI run. tls Issues and PRs related to the tls subsystem. labels Nov 2, 2023
If user uses invalid type for `options.SNICallback` in
TLSSocket(), it's not internal issue of Node.js. So
validateFunction() is more proper than assert().
@deokjinkim
deokjinkimforce-pushed the 231103_tls_snicallback branch from f8d52de to 603d345CompareNovember 2, 2023 16:49
@lpincalpinca added the request-ci Add this label to start a Jenkins CI on a PR. label Nov 3, 2023
@github-actionsgithub-actionsBot removed the request-ci Add this label to start a Jenkins CI on a PR. label Nov 3, 2023
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@deokjinkimdeokjinkim added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. commit-queue Add this label to land a pull request using GitHub Actions. labels Nov 4, 2023
@nodejs-github-botnodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Nov 10, 2023
@nodejs-github-bot
nodejs-github-bot merged commit 609cd7f into nodejs:mainNov 10, 2023
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Landed in 609cd7f

targos pushed a commit that referenced this pull request Nov 11, 2023
If user uses invalid type for `options.SNICallback` in
TLSSocket(), it's not internal issue of Node.js. So
validateFunction() is more proper than assert().
PR-URL: #50530
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
@targostargos mentioned this pull request Nov 12, 2023
targos pushed a commit that referenced this pull request Nov 14, 2023
If user uses invalid type for `options.SNICallback` in
TLSSocket(), it's not internal issue of Node.js. So
validateFunction() is more proper than assert().
PR-URL: #50530
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
UlisesGascon pushed a commit that referenced this pull request Dec 11, 2023
If user uses invalid type for `options.SNICallback` in
TLSSocket(), it's not internal issue of Node.js. So
validateFunction() is more proper than assert().
PR-URL: #50530
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
@UlisesGasconUlisesGascon mentioned this pull request Dec 12, 2023
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

author readyPRs that have at least one approval, no pending requests for changes, and a CI started.needs-ciPRs that need a full CI run.tlsIssues and PRs related to the tls subsystem.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@deokjinkim@nodejs-github-bot@lpinca