tls: fix inconsistent (hostname vs host) #21450
Conversation
Updated error messages and function arguments Refs: nodejs#20892
|
Hi @apapirovski no one reviewed this one yet |
jasnell
added
the
semver-major
|
What's the status on this one? |
| @@ -221,12 +221,12 @@ exports.checkServerIdentity = function checkServerIdentity(hostname, cert) { | |||
| valid = wildcard(cn); | |||
|
|
|||
| if (!valid) | |||
| reason = `Host: ${hostname}. is not cert's CN: ${cn}`; | |||
| reason = `Hostname: ${hostname} is not cert's CN: ${cn}`; | |||
There was a problem hiding this comment.
The removal of the dot misses the, ah, point of unfqdn(), see the comment on line 209. Including the FQDN in the error message is intentional and should not be changed without good reason.
There was a problem hiding this comment.
Ah right, got it, a comment here would be good then because that is most certainly not obvious.
There was a problem hiding this comment.
Maybe wrapping it in quotation marks might help it be mildly more obvious that it's not a typo?
| reason = `Hostname: ${hostname} is not cert's CN: ${cn}`; | |
| reason = `Hostname: "${hostname}." is not cert's CN: ${cn}`; |
|
Given the long time between submission (June 21) and the blocking review (September 26) and now (November 21) the long gap between that and anything happening to move it forward, someone want to make the change and force push to the user's branch just to get this landed? I'm assuming it's a valuable and desirable change. |
|
I'm closing this due to inactivity. Please re-open if needed. |
Updated error messages and function arguments
Refs: #20892
Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passes