tls: simplify getCACertificates() API

Author: haramj

doc/api/tls.md

@@ -2327,12 +2327,10 @@ changes:
   * `type` {string} The type of CA certificates to return. One of `"default"`, `"system"`, `"bundled"`, or `"extra"`.
     **Default:** `"default"`.
   * `format` {string} The format of returned certificates. One of `"string"`, `"buffer"`, or `"x509"`.
-    **Default**: `"string"`.
-    * `"string"`: Returns PEM-encoded strings by default. See `encoding` for DER.
-    * `"buffer"`: Returns an array of certificate data as `Buffer` objects.
+    **Default:** `"string"`.
+    * `"string"`: Returns an array of PEM-encoded certificate strings.
+    * `"buffer"`: Returns an array of certificate data as `Buffer` objects in DER format.
     * `"x509"`: Returns an array of [`X509Certificate`][x509certificate] instances.
-  * `encoding` {string} When `format` is `"string"`, the encoding of the returned strings.
-    One of `"pem"` (PEM-encoded) or `"der"` (base64 DER). **Default:** `"pem"`.
 
 * Returns: {Array}
   An array of certificate data in the specified format (Buffer or X509Certificate).

lib/tls.js

@@ -178,14 +178,10 @@ function getCACertificates(options = {}) {
   const {
     type = 'default',
     format = 'string',
-    encoding = 'pem',
   } = options;
 
   validateString(type, 'type');
   validateOneOf(format, 'format', ['string', 'buffer', 'x509']);
-  if (format === 'string') {
-    validateOneOf(encoding, 'encoding', ['pem', 'der']);
-  }
 
   let certs;
   switch (type) {
@@ -196,37 +192,33 @@ function getCACertificates(options = {}) {
     default: throw new ERR_INVALID_ARG_VALUE('type', type);
   }
 
-  if (format === 'string' && encoding === 'pem') {
-    return certs;
-  }
-
-  if (format === 'buffer' || format === 'x509') {
-    const buffers = certs.map((cert) => {
-      if (Buffer.isBuffer(cert)) return cert;
-      if (typeof cert === 'string') {
-        const base64 = cert
-          .replace(/-----BEGIN CERTIFICATE-----/g, '')
-          .replace(/-----END CERTIFICATE-----/g, '')
-          .replace(/\s+/g, '');
-        return Buffer.from(base64, 'base64');
-      }
+  if (format === 'string') {
+    // Return PEM strings directly
+    return certs.map((cert) => {
+      if (typeof cert === 'string') return cert;
+      if (Buffer.isBuffer(cert)) return cert.toString('ascii');
       throw new ERR_INVALID_ARG_VALUE('cert', cert);
     });
-    return (format === 'buffer') ? buffers : buffers.map((buf) => new X509Certificate(buf));
   }
 
-  return certs.map((cert) => {
+  const buffers = certs.map((cert) => {
+    if (Buffer.isBuffer(cert)) return cert;
     if (typeof cert === 'string') {
       const base64 = cert
         .replace(/-----BEGIN CERTIFICATE-----/g, '')
         .replace(/-----END CERTIFICATE-----/g, '')
         .replace(/\s+/g, '');
-      return Buffer.from(base64, 'base64').toString('base64');
+      return Buffer.from(base64, 'base64');
     }
-    return encoding === 'pem' ? cert.toString('utf8') : cert.toString('base64');
+    throw new ERR_INVALID_ARG_VALUE('cert', cert);
   });
-}
 
+  if (format === 'buffer') {
+    return buffers;
+  }
+
+  return buffers.map((buf) => new X509Certificate(buf));
+}
 
 exports.getCACertificates = getCACertificates;
 

test/parallel/test-tls-get-ca-certificates-bundled.js

@@ -1,6 +1,5 @@
 'use strict';
-// This tests that tls.getCACertificates() returns the bundled
-// certificates correctly.
+// Test that tls.getCACertificates() returns the bundled certificates correctly.
 
 const common = require('../common');
 if (!common.hasCrypto) common.skip('missing crypto');
@@ -9,12 +8,9 @@ const assert = require('assert');
 const tls = require('tls');
 const { assertIsCAArray } = require('../common/tls');
 
-const certs = tls.getCACertificates('bundled');
+const certs = tls.getCACertificates({ type: 'bundled', format: 'string' });
 assertIsCAArray(certs);
 
-// It's the same as tls.rootCertificates - both are
-// Mozilla CA stores across platform.
-assert.strictEqual(certs, tls.rootCertificates);
+assert.deepStrictEqual(certs, tls.rootCertificates);
 
-// It's cached on subsequent accesses.
-assert.strictEqual(certs, tls.getCACertificates('bundled'));
+assert.deepStrictEqual(certs, tls.getCACertificates({ type: 'bundled', format: 'string' }));

test/parallel/test-tls-get-ca-certificates-default.js

@@ -1,7 +1,5 @@
 'use strict';
-
-// This tests that tls.getCACertificates() returns the default
-// certificates correctly.
+// Test that tls.getCACertificates() returns the default certificates correctly.
 
 const common = require('../common');
 if (!common.hasCrypto) common.skip('missing crypto');
@@ -10,11 +8,10 @@ const assert = require('assert');
 const tls = require('tls');
 const { assertIsCAArray } = require('../common/tls');
 
-const certs = tls.getCACertificates();
+const certs = tls.getCACertificates({ format: 'string' });
 assertIsCAArray(certs);
 
-const certs2 = tls.getCACertificates('default');
-assert.strictEqual(certs, certs2);
+const certs2 = tls.getCACertificates({ type: 'default', format: 'string' });
+assert.deepStrictEqual(certs, certs2);
 
-// It's cached on subsequent accesses.
-assert.strictEqual(certs, tls.getCACertificates('default'));
+assert.deepStrictEqual(certs, tls.getCACertificates({ type: 'default', format: 'string' }));

test/parallel/test-tls-get-ca-certificates-system.js

@@ -1,7 +1,6 @@
 'use strict';
 // Flags: --use-system-ca
-// This tests that tls.getCACertificates() returns the system
-// certificates correctly.
+// Test that tls.getCACertificates() returns system certificates correctly.
 
 const common = require('../common');
 if (!common.hasCrypto) common.skip('missing crypto');
@@ -10,23 +9,20 @@ const assert = require('assert');
 const tls = require('tls');
 const { assertIsCAArray } = require('../common/tls');
 
-const systemCerts = tls.getCACertificates('system');
-// Usually Windows come with some certificates installed by default.
-// This can't be said about other systems, in that case check that
-// at least systemCerts is an array (which may be empty).
+const systemCerts = tls.getCACertificates({ type: 'system', format: 'string' });
+
 if (common.isWindows) {
   assertIsCAArray(systemCerts);
 } else {
   assert(Array.isArray(systemCerts));
 }
 
-// When --use-system-ca is true, default is a superset of system
-// certificates.
-const defaultCerts = tls.getCACertificates('default');
+const defaultCerts = tls.getCACertificates({ format: 'string' });
 assert(defaultCerts.length >= systemCerts.length);
 const defaultSet = new Set(defaultCerts);
 const systemSet = new Set(systemCerts);
-assert.deepStrictEqual(defaultSet.intersection(systemSet), systemSet);
+for (const cert of systemSet) {
+  assert(defaultSet.has(cert));
+}
 
-// It's cached on subsequent accesses.
-assert.strictEqual(systemCerts, tls.getCACertificates('system'));
+assert.deepStrictEqual(systemCerts, tls.getCACertificates({ type: 'system', format: 'string' }));

test/parallel/test-tls-get-ca-certificates-x509-option.js

@@ -10,37 +10,29 @@ const { X509Certificate } = require('crypto');
 
 {
   const certs = tls.getCACertificates({ type: 'default', format: 'x509' });
-
-  assert.ok(Array.isArray(certs), 'should return an array');
-  assert.ok(certs.length > 0, 'should return non-empty array');
-
-  for (let i = 0; i < certs.length; i++) {
-    assert.ok(certs[i] instanceof X509Certificate,
-              `cert at index ${i} should be instance of X509Certificate`);
+  assert.ok(Array.isArray(certs));
+  assert.ok(certs.length > 0);
+  for (const cert of certs) {
+    assert.ok(cert instanceof X509Certificate);
   }
-
-  assert.match(certs[0].serialNumber, /^[0-9A-F]+$/i,
-               'serialNumber should be hex string');
 }
 
 {
   const certs = tls.getCACertificates({ type: 'default', format: 'buffer' });
   assert.ok(Array.isArray(certs));
   assert.ok(certs.length > 0);
-
-  for (let i = 0; i < certs.length; i++) {
-    assert.ok(Buffer.isBuffer(certs[i]),
-              `cert at index ${i} should be a Buffer`);
+  for (const cert of certs) {
+    assert.ok(Buffer.isBuffer(cert));
   }
 }
 
 {
   const certs = tls.getCACertificates({ type: 'default' });
   assert.ok(Array.isArray(certs));
   assert.ok(certs.length > 0);
-  for (let i = 0; i < certs.length; i++) {
-    assert.strictEqual(typeof certs[i], 'string');
-    assert.ok(certs[i].includes('-----BEGIN CERTIFICATE-----'));
+  for (const cert of certs) {
+    assert.strictEqual(typeof cert, 'string');
+    assert.ok(cert.includes('-----BEGIN CERTIFICATE-----'));
   }
 }
 
@@ -58,10 +50,8 @@ const { X509Certificate } = require('crypto');
   const certs = tls.getCACertificates({ format: 'buffer' });
   assert.ok(Array.isArray(certs));
   assert.ok(certs.length > 0);
-
-  for (let i = 0; i < certs.length; i++) {
-    assert.ok(Buffer.isBuffer(certs[i]),
-              `cert at index ${i} should be a Buffer`);
+  for (const cert of certs) {
+    assert.ok(Buffer.isBuffer(cert));
   }
 }
 
@@ -74,13 +64,3 @@ const { X509Certificate } = require('crypto');
     message: "The argument 'type' is invalid. Received 'invalid'"
   });
 }
-
-{
-  const certs = tls.getCACertificates({ format: 'string', encoding: 'der' });
-  assert.ok(Array.isArray(certs));
-  assert.ok(certs.length > 0);
-  for (let i = 0; i < certs.length; i++) {
-    assert.strictEqual(typeof certs[i], 'string');
-    assert.ok(/^[A-Za-z0-9+/]+=*$/.test(certs[i]), 'should be base64 string');
-  }
-}