Skip to content

Commit

Permalink
http: add type checking for hostname
Browse files Browse the repository at this point in the history
Add type checking for options.hostname / options.host
Maintains the use of `undefined` and `null` for default `localhost`,
but other falsy values (like `false`, `0` and `NaN`) are rejected.

PR-URL: #12494
Ref: #12488
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Ali Ijaz Sheikh <ofrobots@google.com>
  • Loading branch information
jasnell committed Apr 24, 2017
1 parent b968d58 commit 85a4e25
Show file tree
Hide file tree
Showing 2 changed files with 37 additions and 1 deletion.
11 changes: 10 additions & 1 deletion lib/_http_client.js
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,14 @@ function isInvalidPath(s) {
return false;
}

function validateHost(host, name) {
if (host != null && typeof host !== 'string') {
throw new TypeError(
`"options.${name}" must either be a string, undefined or null`);
}
return host;
}

function ClientRequest(options, cb) {
OutgoingMessage.call(this);

Expand Down Expand Up @@ -123,7 +131,8 @@ function ClientRequest(options, cb) {
this.agent && this.agent.defaultPort;

var port = options.port = options.port || defaultPort || 80;
var host = options.host = options.hostname || options.host || 'localhost';
var host = options.host = validateHost(options.hostname, 'hostname') ||
validateHost(options.host, 'host') || 'localhost';

var setHost = (options.setHost === undefined);

Expand Down
27 changes: 27 additions & 0 deletions test/parallel/test-http-hostname-typechecking.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
'use strict';

const common = require('../common');
const assert = require('assert');
const http = require('http');

// All of these values should cause http.request() to throw synchronously
// when passed as the value of either options.hostname or options.host
const vals = [{}, [], NaN, Infinity, -Infinity, true, false, 1, 0, new Date()];

function errCheck(name) {
return new RegExp(`^TypeError: "options\\.${name}" must either be a ` +
'string, undefined or null$');
}

vals.forEach((v) => {
assert.throws(() => http.request({hostname: v}), errCheck('hostname'));
assert.throws(() => http.request({host: v}), errCheck('host'));
});

// These values are OK and should not throw synchronously
['', undefined, null].forEach((v) => {
assert.doesNotThrow(() => {
http.request({hostname: v}).on('error', common.noop);
http.request({host: v}).on('error', common.noop);
});
});

0 comments on commit 85a4e25

Please sign in to comment.