Skip to content
This repository was archived by the owner on Apr 22, 2023. It is now read-only.

Conversation

jasnell
Copy link
Member

@jasnell jasnell commented Apr 8, 2015

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.

@jasnell
Copy link
Member Author

jasnell commented Apr 14, 2015

@misterdjules @mdawsonibm

@misterdjules
Copy link

Can we add a test to make sure that getLecagyCiphers throws/doesn't throw as expected?

@misterdjules misterdjules added this to the 0.12.3 milestone Apr 14, 2015
@jasnell
Copy link
Member Author

jasnell commented Apr 14, 2015

Done

@jasnell
Copy link
Member Author

jasnell commented Apr 15, 2015

#15445 @misterdjules

@mhdawson
Copy link
Member

One minor nit. Should the tests validate the type of the Error, ie a TypeError or an Error as appropriate ? Otherwise lgtm.

@jasnell
Copy link
Member Author

jasnell commented Apr 22, 2015

/cc @mhdawson @misterdjules ok, please check the two additional commits. if these look good I'll squash the pr down and land.

@jasnell jasnell force-pushed the v0.12-getLegacyCiphers branch from e49de55 to e1d2420 Compare April 23, 2015 15:56
@jasnell
Copy link
Member Author

jasnell commented Apr 23, 2015

@misterdjules @mhdawson ... ok, squashed the commits and made the additional changes discussed


Example:

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Keeping an example (with code formatting) would be nice.

@jasnell jasnell force-pushed the v0.12-getLegacyCiphers branch from 4510ab7 to 9be2395 Compare May 5, 2015 03:41
@misterdjules misterdjules modified the milestones: 0.12.3, 0.12.4 May 14, 2015
jasnell added 2 commits May 18, 2015 08:34
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 <[email protected]>
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)
@jasnell jasnell force-pushed the v0.12-getLegacyCiphers branch from daa4713 to 4e58976 Compare May 18, 2015 16:09
@jasnell
Copy link
Member Author

jasnell commented May 18, 2015

@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.

@misterdjules misterdjules modified the milestones: 0.12.4, 0.12.5 May 25, 2015
@misterdjules misterdjules modified the milestones: 0.12.5, 0.12.6 Jun 22, 2015
@misterdjules misterdjules modified the milestones: 0.12.6, 0.12.7, 0.12.8 Jul 6, 2015
@jasnell
Copy link
Member Author

jasnell commented Aug 27, 2015

Closing this. Taking a different approach.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants