tls: document and test option-less createServer

Either the options or the listener argument to tls.createServer() was optional, but not both. This makes no sense, so align the argument checking and documentation with net.createServer(), which accepts the same option sequence, and which tls.createServer() is modelled on. PR-URL: #9800 Reviewed-By: Roman Reiss <[email protected]> Reviewed-By: Michael Dawson <[email protected]>
nodejs · Dec 17, 2016 · 980acb4 · 980acb4
1 parent 41e1e6e
commit 980acb4
Show file tree

Hide file tree

Showing 3 changed files with 27 additions and 11 deletions.
diff --git a/doc/api/tls.md b/doc/api/tls.md
@@ -959,7 +959,7 @@ publicly trusted list of CAs as given in
 <http://mxr.mozilla.org/mozilla/source/security/nss/lib/ckfw/builtins/certdata.txt>.
 
 
-## tls.createServer(options[, secureConnectionListener])
+## tls.createServer([options][, secureConnectionListener])
 <!-- YAML
 added: v0.3.2
 -->

diff --git a/lib/_tls_wrap.js b/lib/_tls_wrap.js
@@ -745,18 +745,19 @@ TLSSocket.prototype.getProtocol = function() {
 //   "PATH_LENGTH_EXCEEDED", "INVALID_PURPOSE" "CERT_UNTRUSTED",
 //   "CERT_REJECTED"
 //
-function Server(/* [options], listener */) {
-  var options, listener;
+function Server(options, listener) {
+  if (!(this instanceof Server))
+    return new Server(options, listener);
 
-  if (arguments[0] !== null && typeof arguments[0] === 'object') {
-    options = arguments[0];
-    listener = arguments[1];
-  } else if (typeof arguments[0] === 'function') {
+  if (typeof options === 'function') {
+    listener = options;
     options = {};
-    listener = arguments[0];
+  } else if (options == null || typeof options === 'object') {
+    options = options || {};
+  } else {
+    throw new TypeError('options must be an object');
   }
 
-  if (!(this instanceof Server)) return new Server(options, listener);
 
   this._contexts = [];
 

diff --git a/test/parallel/test-tls-no-cert-required.js b/test/parallel/test-tls-no-cert-required.js
@@ -1,4 +1,5 @@
 'use strict';
+var assert = require('assert');
 var common = require('../common');
 
 if (!common.hasCrypto) {
@@ -10,6 +11,20 @@ var tls = require('tls');
 // Omitting the cert or pfx option to tls.createServer() should not throw.
 // AECDH-NULL-SHA is a no-authentication/no-encryption cipher and hence
 // doesn't need a certificate.
-tls.createServer({ ciphers: 'AECDH-NULL-SHA' }).listen(0, function() {
+tls.createServer({ ciphers: 'AECDH-NULL-SHA' })
+  .listen(0, common.mustCall(close));
+
+tls.createServer(assert.fail)
+  .listen(0, common.mustCall(close));
+
+tls.createServer({})
+  .listen(0, common.mustCall(close));
+
+assert.throws(() => tls.createServer('this is not valid'), TypeError);
+
+tls.createServer()
+  .listen(0, common.mustCall(close));
+
+function close() {
   this.close();
-});
+}