Skip to content

Commit

Permalink
errors: make message non-enumerable
Browse files Browse the repository at this point in the history
A error message should always be non-enumerable. This makes sure
that is true for dns errors as well. It also adds another check
in `common.expectsError` to make sure no other regressions are
introduced going forward.

Fixes #19716

PR-URL: #19719
Fixes: #19716
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
  • Loading branch information
BridgeAR committed Apr 5, 2018
1 parent 354849e commit 22da2f7
Show file tree
Hide file tree
Showing 4 changed files with 27 additions and 19 deletions.
37 changes: 18 additions & 19 deletions lib/internal/errors.js
Original file line number Diff line number Diff line change
Expand Up @@ -550,34 +550,33 @@ function exceptionWithHostPort(err, syscall, address, port, additional) {
}

/**
* @param {number|string} err - A libuv error number or a c-ares error code
* @param {number|string} code - A libuv error number or a c-ares error code
* @param {string} syscall
* @param {string} [hostname]
* @returns {Error}
*/
function dnsException(err, syscall, hostname) {
// eslint-disable-next-line no-restricted-syntax
const ex = new Error();
function dnsException(code, syscall, hostname) {
let message;
// FIXME(bnoordhuis) Remove this backwards compatibility nonsense and pass
// the true error to the user. ENOTFOUND is not even a proper POSIX error!
if (err === UV_EAI_MEMORY ||
err === UV_EAI_NODATA ||
err === UV_EAI_NONAME) {
err = 'ENOTFOUND'; // Fabricated error name.
if (code === UV_EAI_MEMORY ||
code === UV_EAI_NODATA ||
code === UV_EAI_NONAME) {
code = 'ENOTFOUND'; // Fabricated error name.
}
if (typeof err === 'string') { // c-ares error code.
const errHost = hostname ? ` ${hostname}` : '';
ex.message = `${syscall} ${err}${errHost}`;
// TODO(joyeecheung): errno is supposed to be a number, like in uvException
ex.code = ex.errno = err;
ex.syscall = syscall;
if (typeof code === 'string') { // c-ares error code.
message = `${syscall} ${code}${hostname ? ` ${hostname}` : ''}`;
} else { // libuv error number
const code = lazyInternalUtil().getSystemErrorName(err);
ex.message = `${syscall} ${code}`;
// TODO(joyeecheung): errno is supposed to be err, like in uvException
ex.code = ex.errno = code;
ex.syscall = syscall;
code = lazyInternalUtil().getSystemErrorName(code);
message = `${syscall} ${code}`;
}
// eslint-disable-next-line no-restricted-syntax
const ex = new Error(message);
// TODO(joyeecheung): errno is supposed to be a number / err, like in
// uvException.
ex.errno = code;
ex.code = code;
ex.syscall = syscall;
if (hostname) {
ex.hostname = hostname;
}
Expand Down
3 changes: 3 additions & 0 deletions test/common/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -691,6 +691,9 @@ exports.expectsError = function expectsError(fn, settings, exact) {
fn = undefined;
}
function innerFn(error) {
const descriptor = Object.getOwnPropertyDescriptor(error, 'message');
assert.strictEqual(descriptor.enumerable,
false, 'The error message should be non-enumerable');
if ('type' in settings) {
const type = settings.type;
if (type !== Error && !Error.isPrototypeOf(type)) {
Expand Down
3 changes: 3 additions & 0 deletions test/parallel/test-dns-lookup.js
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,9 @@ dns.lookup('example.com', common.mustCall((error, result, addressType) => {
assert(error);
assert.strictEqual(tickValue, 1);
assert.strictEqual(error.code, 'ENOENT');
const descriptor = Object.getOwnPropertyDescriptor(error, 'message');
assert.strictEqual(descriptor.enumerable,
false, 'The error message should be non-enumerable');
}));

// Make sure that the error callback is called
Expand Down
3 changes: 3 additions & 0 deletions test/parallel/test-dns-resolveany-bad-ancount.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,9 @@ server.bind(0, common.mustCall(() => {
assert.strictEqual(err.code, 'EBADRESP');
assert.strictEqual(err.syscall, 'queryAny');
assert.strictEqual(err.hostname, 'example.org');
const descriptor = Object.getOwnPropertyDescriptor(err, 'message');
assert.strictEqual(descriptor.enumerable,
false, 'The error message should be non-enumerable');
server.close();
}));
}));

0 comments on commit 22da2f7

Please sign in to comment.