-
-
Notifications
You must be signed in to change notification settings - Fork 957
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
Add pfx
HTTPS option
#1364
Add pfx
HTTPS option
#1364
Conversation
Co-authored-by: Sindre Sorhus <sindresorhus@gmail.com>
Co-authored-by: Sindre Sorhus <sindresorhus@gmail.com>
Co-authored-by: Sindre Sorhus <sindresorhus@gmail.com>
@sindresorhus Thanks for the feedback! I think this is ready for re-review |
Co-authored-by: Sindre Sorhus <sindresorhus@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This needs tests: https://github.com/sindresorhus/got/blob/master/test/https.ts
Here's Node.js tutorial on how to generate pfx: https://nodejs.org/api/tls.html#tls_tls_ssl_concepts
@markdboyd Friendly bump :) |
Yeah, sorry. I had started to work on adding the requested test, but I was having trouble creating a working PFX fixture using the instructions that @szmarczak linked. I can take another crack at it though |
I'll try to do this tomorrow |
This PR is missing the "HTTPS options restore" after the request (which has been recently added). In case of retry a Lines 2391 to 2417 in 38bbb04
|
@markdboyd I've set-up in the "HTTPS Tests" PR a test for the PFX option. Currently it uses the "hidden pfx option" which is deprecated. |
Added `pfx` to the "HTTPS options restore" section
Thanks @Giotino and @szmarczak ! |
Fixes #1306
Adds support for the
pfx
option from TLSSecureContextOptions
to theHTTPSOptions
accepted by got.I tried to follow the existing convention of
passphrase
whereoptions.pfx
andoptions.https.pfx
are both allowed, but usingoptions.pfx
prints a deprecation warning.I also added a basic test of the deprecation warning. I removed this test because it was failing since I wasn't providing a real PFX fixture. It also wasn't a very meaningful test.I wasn't sure what other tests to add but I'm happy to add any that are requested.
Checklist