add comments and more assertions

Author: Ethan-Arrowood

test/parallel/test-https-snicallback-override.js

@@ -37,10 +37,13 @@ const sni = {
 
 describe('Regression test for SNICallback / Certification prioritization issue', () => {
     it('should use certificates from SNICallback', async (t) => {
+        let snicbCount = 0;
         const server = https.createServer({
             cert: root.cert,
             key: root.key,
             SNICallback: (servername, cb) => {
+                snicbCount++;
+                // This returns the secure context generated from the respective certificate
                 cb(null, sni[servername].context)
             }
         }, (req, res) => {
@@ -59,11 +62,29 @@ describe('Regression test for SNICallback / Certification prioritization issue',
         server.listen(PORT);
         await events.once(server, 'listening');
 
+        await assert.doesNotReject(() => new Promise((resolve, reject) => {
+            https.get(`https://127.0.0.1:${PORT}`, { rejectUnauthorized: false, agent }, (response) => {
+                const actualCert = response.socket.getPeerX509Certificate();
+
+                // Assert that raw IP address gets the root cert
+                assert.deepStrictEqual(actualCert.subject, sni['ca5.com'].cert.subject);
+
+                response.on('data', (chunk) => {
+                    assert.strictEqual(chunk.toString(), 'Hello, World!');
+                    resolve();
+                });
+
+                response.on('error', reject);
+            }).on('error', reject);
+        }));
+
         for (const [hostname, { cert: expectedCert }] of Object.entries(sni)) {
             await assert.doesNotReject(() => new Promise((resolve, reject) => {
                 https.get(`https://${hostname}:${PORT}`, { rejectUnauthorized: false, agent }, (response) => {
                     const actualCert = response.socket.getPeerX509Certificate();
 
+                    // This assertion will fail if the certificate on the response does not match the one that is meant to be associated with the hostname
+                    // Currently, the agent1 request will fail as it receives the root cert (ca5) instead.
                     assert.deepStrictEqual(actualCert.subject, expectedCert.subject);
 
                     response.on('data', (chunk) => {
@@ -76,6 +97,9 @@ describe('Regression test for SNICallback / Certification prioritization issue',
             }));
         }
 
+        // SNICallback should only be called for the hostname requests, not the IP one
+        assert.strictEqual(snicbCount, 2);
+
         server.close();
     });
 })