tls: add code for ERR_TLS_INVALID_PROTOCOL_METHOD by sam-github · Pull Request #24729 · nodejs/node (original) (raw)
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Conversation12 Commits1 Checks0 Files changed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
[ Show hidden characters]({{ revealButtonHref }})
Add an error code property to invalid secureProtocol
method
exceptions.
Node errors are supposed to have a .code property, so use the new C++ errors framework.
@joyeecheung Its not clear to me whether this has to be semver-major. I changed the error type from Error
(env->ThrowError()
) to TypeError
, which would be semver-major. The .message
should not have changed. If its possible to make this semver-minor by changing the error type, I will.... then introduce a tiny semver-major PR to change the type to TypeError
for future releases.
If any kind of change like this to the errors is semver-major by definition, then I won't bother splitting it up.
Checklist
make -j4 test
(UNIX), orvcbuild test
(Windows) passes- tests and/or benchmarks are included
- documentation is changed or added
- commit message follows commit guidelines
nodejs-github-bot added c++
Issues and PRs that require attention from people who are familiar with C++.
Issues and PRs related to the crypto subsystem.
labels
sam-github added semver-major
PRs that contain breaking changes and should be released in the next major version.
Issues and PRs related to the tls subsystem.
and removed crypto
Issues and PRs related to the crypto subsystem.
labels
Add an error code property to invalid secureProtocol
method
exceptions.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Regarding semver-ness, I don't feel strongly that this should be semver-major, but it probably doesn't hurt too much to stay on the safe side?
Last ci was green, despite what github shows above (thanks @Trott for restarting it).
Trott added the author ready
PRs that have at least one approval, no pending requests for changes, and a CI started.
label
danbev pushed a commit that referenced this pull request
Add an error code property to invalid secureProtocol
method
exceptions.
PR-URL: #24729 Reviewed-By: Joyee Cheung joyeec9h3@gmail.com Reviewed-By: James M Snell jasnell@gmail.com
refack pushed a commit to refack/node that referenced this pull request
Add an error code property to invalid secureProtocol
method
exceptions.
PR-URL: nodejs#24729 Reviewed-By: Joyee Cheung joyeec9h3@gmail.com Reviewed-By: James M Snell jasnell@gmail.com
OK, not backporting this to 11.x is hurting the backport of TLS1.3 to 11.x.
TLS1.3 tests have many assertions on TLS error's .code
property, which were introduced in #25093, which wasn't backported because this wasn't backported.
I propose backporting this to 11.x as semver-minor by changing the type of the error back to Error
from TypeError
.
The 11.x backport would then flow into 10.x as semver-minor as a pre-req for TLS1.3, so this affects LTS.
@targos @joyeecheung @nodejs/lts do you agree with what I propose?
sam-github added a commit to sam-github/node that referenced this pull request
Add an error code property to invalid secureProtocol
method
exceptions.
PR-URL: nodejs#24729 Reviewed-By: Joyee Cheung joyeec9h3@gmail.com Reviewed-By: James M Snell jasnell@gmail.com
sam-github added a commit to sam-github/node that referenced this pull request
In nodejs#24729, the error was changed to be a TypeError, which is the standard type for this kind of error. However, it was Error in 11.x and earlier, so revert that single aspect, so the backport can be semver-minor.
sam-github added a commit to sam-github/node that referenced this pull request
Add an error code property to invalid secureProtocol
method
exceptions.
PR-URL: nodejs#24729 Reviewed-By: Joyee Cheung joyeec9h3@gmail.com Reviewed-By: James M Snell jasnell@gmail.com
sam-github added a commit to sam-github/node that referenced this pull request
In nodejs#24729, the error was changed to be a TypeError, which is the standard type for this kind of error. However, it was Error in 11.x and earlier, so revert that single aspect, so the backport can be semver-minor.
BethGriggs pushed a commit that referenced this pull request
Add an error code property to invalid secureProtocol
method
exceptions.
Backport-PR-URL: #26951 PR-URL: #24729 Reviewed-By: Joyee Cheung joyeec9h3@gmail.com Reviewed-By: James M Snell jasnell@gmail.com
BethGriggs pushed a commit that referenced this pull request
In #24729, the error was changed to be a TypeError, which is the standard type for this kind of error. However, it was Error in 11.x and earlier, so revert that single aspect, so the backport can be semver-minor.
PR-URL: #26951 Reviewed-By: Rod Vagg rod@vagg.org Reviewed-By: Beth Griggs Bethany.Griggs@uk.ibm.com
codebytere added a commit that referenced this pull request
Notable changes:
- deps: add s390 asm rules for OpenSSL-1.1.1 (Shigeki Ohtsu) #19794
- src: add .code and SSL specific error properties (Sam Roberts) #25093
- tls:
- add --tls-min-v1.2 CLI switch (Sam Roberts) #26951
- supported shared openssl 1.1.0 (Sam Roberts) #26951
- revert default max toTLSv1.2 (Sam Roberts) #26951
- revert change to invalid protocol error type (Sam Roberts) #26951
- support TLSv1.3 (Sam Roberts) #26209
- add code for ERR_TLS_INVALID_PROTOCOL_METHOD (Sam Roberts) #24729
codebytere added a commit that referenced this pull request
Notable changes:
- deps: add s390 asm rules for OpenSSL-1.1.1 (Shigeki Ohtsu) #19794
- src: add .code and SSL specific error properties (Sam Roberts) #25093
- tls:
- add --tls-min-v1.2 CLI switch (Sam Roberts) #26951
- supported shared openssl 1.1.0 (Sam Roberts) #26951
- revert default max toTLSv1.2 (Sam Roberts) #26951
- revert change to invalid protocol error type (Sam Roberts) #26951
- support TLSv1.3 (Sam Roberts) #26209
- add code for ERR_TLS_INVALID_PROTOCOL_METHOD (Sam Roberts) #24729
BethGriggs added a commit that referenced this pull request
Notable changes:
- assert:
- async_hooks:
- bootstrap
- make Buffer and process non-enumerable (Ruben Bridgewater) #24874
- buffer:
- child_process:
- console:
- don't use ANSI escape codes when TERM=dumb (Vladislav Kaminsky) #26261
- crypto:
- deps:
- silence irrelevant V8 warning (Michaël Zasso) #26685
- update postmortem metadata generation script (cjihrig) #26685
- V8: un-cherry-pick bd019bd (Refael Ackermann) #26685
- V8: cherry-pick 6 commits (Michaël Zasso) #26685
- V8: cherry-pick d82c9af (Anna Henningsen) #26685
- V8: cherry-pick e5f01ba (Anna Henningsen) #26685
- V8: cherry-pick d5f08e4 (Anna Henningsen) #26685
- V8: cherry-pick 6b09d21 (Anna Henningsen) #26685
- V8: cherry-pick f0bb5d2 (Anna Henningsen) #26685
- V8: cherry-pick 5b0510d (Anna Henningsen) #26685
- V8: cherry-pick 91f0cd0 (Anna Henningsen) #26685
- V8: cherry-pick 392316d (Anna Henningsen) #26685
- V8: cherry-pick 2f79d68 (Anna Henningsen) #26685
- sync V8 gypfiles with 7.4 (Ujjwal Sharma) #26685
- update V8 to 7.4.288.13 (Ujjwal Sharma) #26685
- bump minimum icu version to 63 (Ujjwal Sharma) #25852
- silence irrelevant V8 warnings (Michaël Zasso) #25852
- V8: cherry-pick 7803fa6 (Jon Kunkee) #25852
- V8: cherry-pick 58cefed (Jon Kunkee) #25852
- V8: cherry-pick d3308d0 (Michaël Zasso) #25852
- V8: cherry-pick 74571c8 (Michaël Zasso) #25852
- cherry-pick fc0ddf5 from upstream V8 (Anna Henningsen) #25852
- sync V8 gypfiles with 7.3 (Ujjwal Sharma) #25852
- sync V8 gypfiles with 7.2 (Michaël Zasso) #25852
- update V8 to 7.3.492.25 (Michaël Zasso) #25852
- add s390 asm rules for OpenSSL-1.1.1 (Shigeki Ohtsu) #19794
- sync V8 gypfiles with 7.1 (Refael Ackermann) #23423
- update V8 to 7.1.302.28 (Michaël Zasso) #23423
- doc:
- errors:
- update error name (Ruben Bridgewater) #26738
- fs:
- http:
- lib:
- move DEP0021 to end of life (cjihrig) #27127
- remove Atomics.wake (Gus Caplan) #27033
- validate Error.captureStackTrace() calls (Ruben Bridgewater) #26738
- refactor Error.captureStackTrace() usage (Ruben Bridgewater) #26738
- move DTRACE_* probes out of global scope (James M Snell) #26541
- deprecate _stream_wrap (Sam Roberts) [#26245] (#26245)
- don't use
util.inspect()
internals (Ruben Bridgewater) #24971 - improve error message for MODULE_NOT_FOUND (Ali Ijaz Sheikh) #25690
- requireStack property for MODULE_NOT_FOUND (Ali Ijaz Sheikh) #25690
- move DEP0029 to end of life (cjihrig) #25377
- move DEP0028 to end of life (cjihrig) #25377
- move DEP0027 to end of life (cjihrig) #25377
- move DEP0026 to end of life (cjihrig) #25377
- move DEP0023 to end of life (cjihrig) #25280
- move DEP0006 to end of life (cjihrig) #25279
- remove unintended access to deps/ (Anna Henningsen) #25138
- move DEP0120 to end of life (cjihrig) #24862
- use ES6 class inheritance style (Ruben Bridgewater) #24755
- remove
inherits()
usage (Ruben Bridgewater) #24755
- module:
- n-api:
- remove code from error name (Ruben Bridgewater) #26738
- net:
- net,http2:
- merge setTimeout code (ZYSzys) #25084
- os:
- implement os.type() using uv_os_uname() (cjihrig) #25659
- process:
- readline:
- support TERM=dumb (Vladislav Kaminsky) #26261
- repl:
- src:
- remove unused INT_MAX constant (Sam Roberts) #27078
- update NODE_MODULE_VERSION to 72 (Ujjwal Sharma) #26685
- remove
AddPromiseHook()
(Anna Henningsen) #26574 - update NODE_MODULE_VERSION to 71 (Michaël Zasso) #25852
- clean up MultiIsolatePlatform interface (Anna Henningsen) #26384
- properly configure default heap limits (Ali Ijaz Sheikh) #25576
- remove icuDataDir from node config (GauthamBanasandra) #24780
- explicitly allow JS in ReadHostObject (Yang Guo) #23423
- update postmortem constant (cjihrig) #23423
- update NODE_MODULE_VERSION to 68 (Michaël Zasso) #23423
- tls:
- support TLSv1.3 (Sam Roberts) #26209
- return correct version from getCipher() (Sam Roberts) #26625
- check arg types of renegotiate() (Sam Roberts) #25876
- add code for ERR_TLS_INVALID_PROTOCOL_METHOD (Sam Roberts) #24729
- emit a warning when servername is an IP address (Rodger Combs) #23329
- disable TLS v1.0 and v1.1 by default (Ben Noordhuis) #23814
- remove unused arg to createSecureContext() (Sam Roberts) #24241
- deprecate Server.prototype.setOptions() (cjihrig) #23820
- load NODE_EXTRA_CA_CERTS at startup (Ouyang Yadong) #23354
- util:
- change inspect compact and breakLength default (Ruben Bridgewater) #27109
- improve inspect edge cases (Ruben Bridgewater) #27109
- only the first line of the error message (Simon Zünd) #26685
- don't set the prototype of callbackified functions (Ruben Bridgewater) #26893
- rename callbackified function (Ruben Bridgewater) #26893
- increase function length when using
callbackify()
(Ruben Bridgewater) #26893 - prevent tampering with internals in
inspect()
(Ruben Bridgewater) #26577 - fix proxy inspection (Ruben Bridgewater) #26241
- prevent leaking internal properties (Ruben Bridgewater) #24971
- protect against monkeypatched Object prototype for inspect() (Rich Trott) #25953
- treat format arguments equally (Roman Reiss) #23162
- win, fs:
- detect if symlink target is a directory (Bartosz Sosnowski) #23724
- zlib:
PR-URL: #26930
BethGriggs added a commit that referenced this pull request
Notable changes:
- assert:
- async_hooks:
- bootstrap: make Buffer and process non-enumerable (Ruben Bridgewater) #24874
- buffer:
- child_process:
- console:
- don't use ANSI escape codes when
TERM=dumb
(Vladislav Kaminsky) #26261
- don't use ANSI escape codes when
- crypto:
- remove legacy native handles (Tobias Nießen) #27011
- decode missing passphrase errors (Tobias Nießen) #25208
- remove
Cipher.setAuthTag()
andDecipher.getAuthTag()
(Tobias Nießen) #26249 - remove deprecated
crypto._toBuf()
(Tobias Nießen) #25338 - set
DEFAULT\_ENCODING
property to non-enumerable (Antoine du Hamel) #23222
- deps:
- errors:
- update error name (Ruben Bridgewater) #26738
- fs:
- http:
- lib:
- module:
- remove unintended access to deps/ (Anna Henningsen) #25138
- improve error message for MODULE_NOT_FOUND (Ali Ijaz Sheikh) #25690
- requireStack property for MODULE_NOT_FOUND (Ali Ijaz Sheikh) #25690
- remove dead code (Ruben Bridgewater) #26983
- make
require('.')
never resolve outside the current directory (Ruben Bridgewater) #26973 - throw an error for invalid package.json main entries (Ruben Bridgewater) #26823
- don't search in
require.resolve.paths
(cjihrig) #23683
- net:
- os:
- process:
- readline:
- support TERM=dumb (Vladislav Kaminsky) #26261
- repl:
- src:
- remove unused
INT_MAX
constant (Sam Roberts) #27078 - update
NODE_MODULE_VERSION
to 72 (Ujjwal Sharma) #26685 - remove
AddPromiseHook()
(Anna Henningsen) #26574 - clean up
MultiIsolatePlatform
interface (Anna Henningsen) #26384 - properly configure default heap limits (Ali Ijaz Sheikh) #25576
- remove
icuDataDir
from node config (GauthamBanasandra) #24780
- remove unused
- tls:
- support TLSv1.3 (Sam Roberts) #26209
- return correct version from
getCipher()
(Sam Roberts) #26625 - check arg types of renegotiate() (Sam Roberts) #25876
- add code for
ERR_TLS_INVALID_PROTOCOL_METHOD
(Sam Roberts) #24729 - emit a warning when servername is an IP address (Rodger Combs) #23329
- disable TLS v1.0 and v1.1 by default (Ben Noordhuis) #23814
- remove unused arg to createSecureContext() (Sam Roberts) #24241
- deprecate
Server.prototype.setOptions()
(cjihrig) #23820 - load
NODE_EXTRA_CA_CERTS
at startup (Ouyang Yadong) #23354
- util:
- remove
util.print()
,util.puts()
,util.debug()
andutil.error()
(cjihrig) #25377 - change inspect compact and breakLength default (Ruben Bridgewater) #27109
- improve inspect edge cases (Ruben Bridgewater) #27109
- only the first line of the error message (Simon Zünd) #26685
- don't set the prototype of callbackified functions (Ruben Bridgewater) #26893
- rename callbackified function (Ruben Bridgewater) #26893
- increase function length when using
callbackify()
(Ruben Bridgewater) #26893 - prevent tampering with internals in
inspect()
(Ruben Bridgewater) #26577 - prevent Proxy traps being triggered by
.inspect()
(Ruben Bridgewater) #26241 - prevent leaking internal properties (Ruben Bridgewater) #24971
- protect against monkeypatched Object prototype for inspect() (Rich Trott) #25953
- treat format arguments equally (Roman Reiss) #23162
- remove
- win, fs:
- detect if symlink target is a directory (Bartosz Sosnowski) #23724
- zlib:
PR-URL: #26930
BethGriggs added a commit that referenced this pull request
Notable changes:
- assert:
- async_hooks:
- bootstrap: make Buffer and process non-enumerable (Ruben Bridgewater) #24874
- buffer:
- child_process:
- console:
- don't use ANSI escape codes when
TERM=dumb
(Vladislav Kaminsky) #26261
- don't use ANSI escape codes when
- crypto:
- remove legacy native handles (Tobias Nießen) #27011
- decode missing passphrase errors (Tobias Nießen) #25208
- remove
Cipher.setAuthTag()
andDecipher.getAuthTag()
(Tobias Nießen) #26249 - remove deprecated
crypto._toBuf()
(Tobias Nießen) #25338 - set
DEFAULT\_ENCODING
property to non-enumerable (Antoine du Hamel) #23222
- deps:
- errors:
- update error name (Ruben Bridgewater) #26738
- fs:
- http:
- lib:
- module:
- remove unintended access to deps/ (Anna Henningsen) #25138
- improve error message for MODULE_NOT_FOUND (Ali Ijaz Sheikh) #25690
- requireStack property for MODULE_NOT_FOUND (Ali Ijaz Sheikh) #25690
- remove dead code (Ruben Bridgewater) #26983
- make
require('.')
never resolve outside the current directory (Ruben Bridgewater) #26973 - throw an error for invalid package.json main entries (Ruben Bridgewater) #26823
- don't search in
require.resolve.paths
(cjihrig) #23683
- net:
- os:
- process:
- readline:
- support TERM=dumb (Vladislav Kaminsky) #26261
- repl:
- src:
- remove unused
INT_MAX
constant (Sam Roberts) #27078 - update
NODE_MODULE_VERSION
to 72 (Ujjwal Sharma) #26685 - remove
AddPromiseHook()
(Anna Henningsen) #26574 - clean up
MultiIsolatePlatform
interface (Anna Henningsen) #26384 - properly configure default heap limits (Ali Ijaz Sheikh) #25576
- remove
icuDataDir
from node config (GauthamBanasandra) #24780
- remove unused
- tls:
- support TLSv1.3 (Sam Roberts) #26209
- return correct version from
getCipher()
(Sam Roberts) #26625 - check arg types of renegotiate() (Sam Roberts) #25876
- add code for
ERR_TLS_INVALID_PROTOCOL_METHOD
(Sam Roberts) #24729 - emit a warning when servername is an IP address (Rodger Combs) #23329
- disable TLS v1.0 and v1.1 by default (Ben Noordhuis) #23814
- remove unused arg to createSecureContext() (Sam Roberts) #24241
- deprecate
Server.prototype.setOptions()
(cjihrig) #23820 - load
NODE_EXTRA_CA_CERTS
at startup (Ouyang Yadong) #23354
- util:
- remove
util.print()
,util.puts()
,util.debug()
andutil.error()
(cjihrig) #25377 - change inspect compact and breakLength default (Ruben Bridgewater) #27109
- improve inspect edge cases (Ruben Bridgewater) #27109
- only the first line of the error message (Simon Zünd) #26685
- don't set the prototype of callbackified functions (Ruben Bridgewater) #26893
- rename callbackified function (Ruben Bridgewater) #26893
- increase function length when using
callbackify()
(Ruben Bridgewater) #26893 - prevent tampering with internals in
inspect()
(Ruben Bridgewater) #26577 - prevent Proxy traps being triggered by
.inspect()
(Ruben Bridgewater) #26241 - prevent leaking internal properties (Ruben Bridgewater) #24971
- protect against monkeypatched Object prototype for inspect() (Rich Trott) #25953
- treat format arguments equally (Roman Reiss) #23162
- remove
- win, fs:
- detect if symlink target is a directory (Bartosz Sosnowski) #23724
- zlib:
PR-URL: #26930
This was referenced
Apr 23, 2019
codebytere added a commit that referenced this pull request
Notable changes:
- deps: add s390 asm rules for OpenSSL-1.1.1 (Shigeki Ohtsu) #19794
- src: add .code and SSL specific error properties (Sam Roberts) #25093
- tls:
- add --tls-min-v1.2 CLI switch (Sam Roberts) #26951
- supported shared openssl 1.1.0 (Sam Roberts) #26951
- revert default max toTLSv1.2 (Sam Roberts) #26951
- revert change to invalid protocol error type (Sam Roberts) #26951
- support TLSv1.3 (Sam Roberts) #26209
- add code for ERR_TLS_INVALID_PROTOCOL_METHOD (Sam Roberts) #24729
PR-URL: #27314
codebytere added a commit that referenced this pull request
Notable changes:
- deps: add s390 asm rules for OpenSSL-1.1.1 (Shigeki Ohtsu) #19794
- src: add .code and SSL specific error properties (Sam Roberts) #25093
- tls:
- add --tls-min-v1.2 CLI switch (Sam Roberts) #26951
- supported shared openssl 1.1.0 (Sam Roberts) #26951
- revert default max toTLSv1.2 (Sam Roberts) #26951
- revert change to invalid protocol error type (Sam Roberts) #26951
- support TLSv1.3 (Sam Roberts) #26209
- add code for ERR_TLS_INVALID_PROTOCOL_METHOD (Sam Roberts) #24729
PR-URL: #27314
Labels
PRs that have at least one approval, no pending requests for changes, and a CI started.
Issues and PRs that require attention from people who are familiar with C++.
PRs that contain breaking changes and should be released in the next major version.
Issues and PRs related to the tls subsystem.