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

Support path or buffer inputs to the x509 verify method #44

Closed
wants to merge 8 commits into from
Closed

Support path or buffer inputs to the x509 verify method #44

wants to merge 8 commits into from

Conversation

astitt-ripple
Copy link
Contributor

For your consideration, allows for the use case where it is more convenient to use certs and/or CAs from memory than from files.

@yorkie
Copy link
Collaborator

yorkie commented Jan 11, 2017

@astitt-ripple As you said before, we should handle the buffer and path in JavaScript land, keep using memory at the C++ bridge code.

@astitt-ripple
Copy link
Contributor Author

Any idea why travis-ci fails? I cannot see the logs, and npm test is passing in my environment.

@Southern
Copy link
Owner

Travis tests are failing for 0.10 and 0.12:

/home/travis/build/Southern/node-x509/index.js:68
  if (String(certPathOrString).startsWith('-----BEGIN')) {
                               ^
TypeError: Object /home/travis/build/Southern/node-x509/test/certs/enduser-example.com.crt has no method 'startsWith'
    at Object.exports.verify (/home/travis/build/Southern/node-x509/index.js:68:32)
    at x509_verify_test (/home/travis/build/Southern/node-x509/test/test.js:26:8)
    at Object.<anonymous> (/home/travis/build/Southern/node-x509/test/test.js:32:1)
    at Module._compile (module.js:456:26)
    at Object.Module._extensions..js (module.js:474:10)
    at Module.load (module.js:356:32)
    at Function.Module._load (module.js:312:12)
    at Function.Module.runMain (module.js:497:10)
    at startup (node.js:119:16)
    at node.js:945:3

@astitt-ripple
Copy link
Contributor Author

astitt-ripple commented Jan 17, 2017

Ah I see now, thank you @Southern! I was able to reproduce the error locally, and copied in the polyfill from the mozilla developer portal.

@yorkie I've updated the code to read the certs and CABundle in JS instead, and removed the corresponding C++ code

edit: identified a memory leak in the ca buffer code earlier today, will have a fix soon. i apologize if this has caused any inconvenience.

@Southern
Copy link
Owner

Southern commented Feb 6, 2017

@astitt-ripple were you able to get this working? Looks like the tests passed when they were ran last.

@astitt-ripple
Copy link
Contributor Author

@Southern I was able to get it working 👍 There were some memory leak issues I found/resolved during testing as well. I can re-open a PR if this is something you'd consider merging.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants