fix(@schematics/update): Allow usage strict SSL configuration #838
Conversation
| @@ -85,6 +85,7 @@ export function getNpmPackageJson( | |||
| return concat( | |||
| getNpmConfigOption('proxy'), | |||
| getNpmConfigOption('https-proxy'), | |||
| getNpmConfigOption('strict-ssl'), | |||
There was a problem hiding this comment.
Is getNpmConfigOption('strict-ssl') provides true default when strict-ssl is missing in config?
| @@ -95,6 +96,9 @@ export function getNpmPackageJson( | |||
| http: options[0], | |||
| https: options[1], | |||
| }, | |||
| ssl: { | |||
| strict: options[2], | |||
There was a problem hiding this comment.
This will give you the string value instead of the boolean. options[2] === 'true' would be more appropriate.
There was a problem hiding this comment.
Since npm-registry-client's config requires ssl.strict to be a boolean and npm's default config has strict-ssl set to true (https://docs.npmjs.com/misc/config#strict-ssl), the exec npm get strict-ssl in getNpmConfigOption function should yield true if not explicitly set.
options[2] === 'true' should suffice as @alvaro450 mentioned above.
| @@ -95,6 +96,9 @@ export function getNpmPackageJson( | |||
| http: options[0], | |||
| https: options[1], | |||
| }, | |||
| ssl: { | |||
| strict: options[2], | |||
There was a problem hiding this comment.
Since npm-registry-client's config requires ssl.strict to be a boolean and npm's default config has strict-ssl set to true (https://docs.npmjs.com/misc/config#strict-ssl), the exec npm get strict-ssl in getNpmConfigOption function should yield true if not explicitly set.
options[2] === 'true' should suffice as @alvaro450 mentioned above.
| @@ -101,6 +102,9 @@ export function getNpmPackageJson( | |||
| http: options[0], | |||
| https: options[1], | |||
| }, | |||
| ssl: { | |||
| strict: options[2] === 'true', | |||
There was a problem hiding this comment.
If it is undefined it should stay as undefined to pick up the default.
There was a problem hiding this comment.
undefined will cause strict: false. strict is default to true. The key should go there only if strict is false
if(options[2]==='false') {
opts = {...opts, {ssl: {strict: false}}}
}
ssl stuff should be maximally explicit
There was a problem hiding this comment.
Actually, undefined will cause it to be true. It is explicitly handled: https://github.com/npm/npm-registry-client/blob/latest/index.js#L32
However, there is not much downside to being explicit here as well. I'd probably go with something similar to the following:
...(options[2] === 'false' ? { strict: false } : (options[2] === 'true' ? { strict: true } : {})),
There was a problem hiding this comment.
Addressed and updated
Fixes angular/angular-cli#10596