tls: throw an error on getLegacyCipher#14573
Conversation
|
@misterdjules @mdawsonibm |
|
Can we add a test to make sure that getLecagyCiphers throws/doesn't throw as expected? |
|
Done |
|
One minor nit. Should the tests validate the type of the Error, ie a TypeError or an Error as appropriate ? Otherwise lgtm. |
|
/cc @mhdawson @misterdjules ok, please check the two additional commits. if these look good I'll squash the pr down and land. |
e49de55 to
e1d2420
Compare
|
@misterdjules @mhdawson ... ok, squashed the commits and made the additional changes discussed |
doc/api/tls.markdown
Outdated
There was a problem hiding this comment.
Keeping an example (with code formatting) would be nice.
4510ab7 to
9be2395
Compare
Disable RC4 in the default cipher list Add the `--cipher-list` command line switch and `NODE_CIPHER_LIST` environment variable to completely override the default cipher list. Add the `--enable-legacy-cipher-list` and `NODE_LEGACY_CIPHER_LIST` environment variable to selectively enable the default cipher list from previous node.js releases. Reviewed-By: James M Snell <jasnell@gmail.com> PR-URL: nodejs#14414
Add the `--cipher-list` and `--enable-legacy-cipher-list` command line switches. Add the `NODE_CIPHER_LIST` and `NODE_LEGACY_CIPHER_LIST` environment variables. Add the getLegacyCiphers method to tls.js Add the test/simple/test-tls-cipher-list.js test case (this is a reworked and squashed recommit of the work previously backed back out of v0.12 for the v0.12.3 release)
daa4713 to
4e58976
Compare
|
@misterdjules @mhdawson ... ok, the PR has been rebased following the v0.12.3 cut and squashed down to just two commits against the v0.12 branch. @misterdjules please review and let me know if you feel this is ready to land. |
|
Closing this. Taking a different approach. |
throw an error on unknown getLegacyCipher input
rather than returning the default. Makes the
behavior more explicit and matches up with the
behavior on the command line switches.