Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

doc: updated tls docs for tls socket subjectaltname #27734

Closed
wants to merge 4 commits into from
Closed

doc: updated tls docs for tls socket subjectaltname #27734

wants to merge 4 commits into from

Conversation

dannyb648
Copy link
Contributor

The documentation for tlsSocket.getPeerCertificate() stated that the
certificate object returned subjectaltname as an array
however it actually returned a concatenated list of strings as a single string
docs have been updated to reflect this

Fixes: #27721

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • documentation is changed or added
  • commit message follows commit guidelines

The documentation for tlsSocket.getPeerCertificate() stated that the
certificate object returned subjectaltname as an array
however it actually returned a concatenated list of strings as a single string
docs have been updated to reflect this

Fixes: #27721
@nodejs-github-bot nodejs-github-bot added doc Issues and PRs related to the documentations. tls Issues and PRs related to the tls subsystem. labels May 16, 2019
doc/api/tls.md Outdated
@@ -917,7 +917,7 @@ certificate.
It is returned as a `:` separated hexadecimal string. Example:
`'2A:7A:C2:DD:...'`.
* `ext_key_usage` {Array} (Optional) The extended key usage, a set of OIDs.
* `subjectaltname` {Array} (Optional) An array of names for the subject, an
* `subjectaltname` {string} (Optional) A string containing concatenated names for the subject, an
Copy link
Member

Choose a reason for hiding this comment

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

I think the linter is not happy with this long line.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

@dannyb648
Copy link
Contributor Author

Original Commit message was invalid, closing and resubmitting as I cant go back and fix that.

@dannyb648 dannyb648 closed this May 17, 2019
@sam-github
Copy link
Contributor

Anything about this PR, including the commit message, can be rewritten, it is not necessary to open a new PR.

Can you reopen this, to preserve conversation and approvals, and then describe what needs to be fixed?

I can either explain how, if you would like to, or I can force push to your branch to fix it for you.

@dannyb648
Copy link
Contributor Author

I didnt think of that, completely agree about keeping this PR.

I deleted my remote fork of node.js - ive still got the local version, however reforking and pushing my local changes to the new remote fork isnt letting me open this PR? What would you suggest as steps forward. First time contributing to a Open Source project so still learning the ropes.

@sam-github
Copy link
Contributor

If you deleted the fork, then yes, you may have to re-PR the change.

I can't see the option, but since you made this PR, if you look up through the history, you may see a button somewhere on this page that says something about "undeleting" the branch. If so, try that, if not, you'll have to push your local branch to your new fork.

@dannyb648
Copy link
Contributor Author

Recreated in #27757 since Fork was deleted, pushed local copy to new branch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc Issues and PRs related to the documentations. tls Issues and PRs related to the tls subsystem.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

API docs: type for TLS Certificate's subjectaltname field is wrong.
4 participants