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

Incorrect function #15

Open
YuryStrozhevsky opened this issue Jun 20, 2018 · 4 comments
Open

Incorrect function #15

YuryStrozhevsky opened this issue Jun 20, 2018 · 4 comments

Comments

@YuryStrozhevsky
Copy link

Seems you at least incorrectly named this function. And in order to get serialNumber would could use this code:

import { bufferToHexCodes } from "pvutils";
const serialNumber = bufferToHexCodes(certificate.serialNumber.valueBlock.valueHex);

or this one:

const serialNumber = certificate.serialNumber.valueBlock.toString(); // for decimal representation, even for huge serial numbers
@YuryStrozhevsky
Copy link
Author

And having this is not a good idea :)

    getIssuer() {
        return this._cert.issuer.typesAndValues[0].value.valueBlock.value;
    }

It is not a good idea because no one could be 100% sure that first element in typesAndValues would be able to be represented as string. Yes, in most cases you would get no problems. But anyway I do suggest you to at least search for commonName OID and return its string represemtation in your getIssuer function.

@apowers313
Copy link
Collaborator

Thanks for the review and the feedback. I'll go back and make your recommended changes.

I have to admit that trying to figure out how to interpret values from the certs has been a trial-and-error process. Half of that has been trying to understand the data structure created by pki.js -- it's not entirely clear to me how to get to the values for the different fields -- although I think I have learned quite a bit (or maybe developed strange coping mechanisms) about that along the way. If / when you write documentation for pki.js this might be one of the good initial areas to focus on.

@YuryStrozhevsky
Copy link
Author

Documentation for such a huge code would be a nightmare :) But I will consider this for sure.

@JamesCullum
Copy link
Member

Looks like a good first issue for contributors - waiting for a PR

JamesCullum added a commit that referenced this issue Oct 6, 2023
* Add fixes for getIssuer and getSerial (#15)

* Add search for commonName in getIssuer
* Properly return serial number from _cert in getSerial

* Use array.prototype.find instead of a for loop to find commonName

* Remove static and private from searchForCommonName

* Use for of loop in searchForCommonName

* Add compatability to getSerial

* Use double quotes in getSerial

* Fix typo and suggest using getSerial("v2") in deprecation warning

* Use cert.getCommonName to get serial in CertManager

* Remove unnecessary param from getCommonName

* Change names of variables / functions in CertManager to indicate that common name is used as a key instead of the serial number

---------

Co-authored-by: James Cullum (Pseudonym) <JamesCullum@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants