doc: Improve checkServerIdentity docs #17203
Conversation
nodejs-github-bot
added
doc
|
See also #11467 |
doc/api/tls.md
Outdated
| @@ -792,8 +792,9 @@ changes: | |||
| * `servername`: {string} Server name for the SNI (Server Name Indication) TLS | |||
| extension. | |||
| * `checkServerIdentity(servername, cert)` {Function} A callback function | |||
| to be used when checking the server's hostname against the certificate. | |||
| This should throw an error if verification fails. The method should return | |||
| to be used (instead of the builtin {tls.checkServerIdentity} function) | |||
There was a problem hiding this comment.
Can you please change this line to this?:
to be used instead of
tls.checkServerIdentity()
Backticks are appropriate here. Curly braces are for types. Thanks!
There was a problem hiding this comment.
Ahh. That makes sense. Thanks for the comment!
Fixed :)
Hannes-Magnusson-CK
force-pushed
the
patch-2
branch
from
4bf98c7
to
ea0cd07
Compare
doc/api/tls.md
Outdated
| @@ -792,8 +792,9 @@ changes: | |||
| * `servername`: {string} Server name for the SNI (Server Name Indication) TLS | |||
| extension. | |||
| * `checkServerIdentity(servername, cert)` {Function} A callback function | |||
| to be used when checking the server's hostname against the certificate. | |||
| This should throw an error if verification fails. The method should return | |||
| to be used (instead of the builtin `tls.checkServerIdentity` function) | |||
There was a problem hiding this comment.
Tiny nit: can you use tls.checkServerIdentity()? It seems to be the dominant style.
There was a problem hiding this comment.
of course! :) Updated
Hannes-Magnusson-CK
force-pushed
the
patch-2
branch
from
ea0cd07
to
15a4b95
Compare
|
Probably a dumb question, I'm new around here =): What makes those export more public then say https://github.com/nodejs/node/blob/master/lib/tls.js#L168 ? If thats purely source code readability, should I document the checkServerIdentity function? |
|
@Hannes-Magnusson-CK The comment is a bit misguided / misleading, it should just be removed. You could read it as "these are the public and documented APIs" but some of the exports elsewhere are mentioned in the docs too, if only in passing. |
|
Ok. Thanks @bnoordhuis ! |
The funciton was added in eb2ca10
|
Added a commit documenting |
| @@ -236,7 +236,6 @@ exports.parseCertString = internalUtil.deprecate( | |||
| 'Please use querystring.parse() instead.', | |||
| 'DEP0076'); | |||
|
|
|||
| // Public API | |||
There was a problem hiding this comment.
Its a misleading comment. These aren't the only exported functions.
addaleax
added
the
author ready
PR-URL: #17203 Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net>
addaleax
removed
the
author ready
PR-URL: #17203 Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net>
PR-URL: #17203 Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net>
PR-URL: #17203 Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net>
PR-URL: #17203 Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net>
PR-URL: #17203 Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net>
Checklist
Affected core subsystem(s)
tls doc