With this PR I also wanted to add the documentation described in #1191

2 out of the 3 current HTTPS test are broken, the first 2 make plain HTTP requests.

You should have used got.secure instead of got

2 out of the 3 current HTTPS test are broken, the first 2 make plain HTTP requests.

You should have used got.secure instead of got

Could you fix that please? 🙏

2 out of the 3 current HTTPS test are broken, the first 2 make plain HTTP requests.
You should have used got.secure instead of got

Could you fix that please? 🙏

Yeah, I'm working on it

I was working on a test for checkServerIdentity and i discovered that it's not called when connecting to 127.0.0.1 (and other localhost addresses)...

const tls = require('tls');

tls.connect({
  host: '127.0.0.1',
  port: 49637,
  checkServerIdentity: (hostname, certificate) => {
    // This is never called for localhost addresses
  }
})

Of course there's no documentation by Node.JS, there's only a comment in an example:

// Necessary only if the server's cert isn't for "localhost".
checkServerIdentity: () => { return null; },

I've to keep in mind this when writing the documentation, also the test need to be performed using an external service, we can use https://badssl.com/

Maybe there's a servername option missing? Try setting it to localhost.

Or maybe try setting host to localhost instead of 127.0.0.1...

Maybe there's a servername option missing? Try setting it to localhost.
Or maybe try setting host to localhost instead of 127.0.0.1...

I've tried all of them, same result

I made a mistake, it seems that checkServerIdentity is only called if the certificate is valid. That's why localhost was not working (I was testing without the test CA). I'm going to add some tests.

While Options extends SecureContextOptions these features are not documented.

For example key, cert and ca useful for using a client certificate, or other params to tweak the TLS options.

My idea would be to add an HTTPS section to the documentation.

I agree.

This is a first draft of the HTTPS documentation, there're some other parameter. The params that I've included there are the most used.

Since key, cert and passphrase are always used together I grouped the examples in a single block.

@sindresorhus If you want to have different names (from the tls module) for the properties related to HTTPS I think that Options should stop extending SecureContextOptions and start defining its HTTPS properties and then translate them before the request.
I'm working on a commit with an example of that.

"Deprecation Warning" tests must be serial or else they'll steal each other process events.

There are a few more unresolved inline comments.

There are a few more unresolved inline comments.

Sorry, I missed them, I was watching only the "Conversation" tab on Github.

This looks great now. Thanks for your hard work on this 🙌