From 98f5b17ee1922575f5337217c927f4eb3a3fd227 Mon Sep 17 00:00:00 2001
From: Ruben Bridgewater <ruben@bridgewater.de>
Date: Sun, 1 Apr 2018 07:38:47 +0200
Subject: [PATCH] errors: make message non-enumerable

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

Backport-PR-URL: https://github.com/nodejs/node/pull/19191
PR-URL: https://github.com/nodejs/node/pull/19719
Fixes: https://github.com/nodejs/node/issues/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>
---
 lib/internal/errors.js                        | 36 +++++++++----------
 test/common/index.js                          |  3 ++
 test/parallel/test-dns-lookup.js              |  3 ++
 .../test-dns-resolveany-bad-ancount.js        |  3 ++
 4 files changed, 27 insertions(+), 18 deletions(-)

diff --git a/lib/internal/errors.js b/lib/internal/errors.js
index 93418006a7ac95..e62ce3fb22759a 100644
--- a/lib/internal/errors.js
+++ b/lib/internal/errors.js
@@ -204,33 +204,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) {
-  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;
   }
diff --git a/test/common/index.js b/test/common/index.js
index cb82cd6a93ee3a..41d4534307c7d6 100644
--- a/test/common/index.js
+++ b/test/common/index.js
@@ -692,6 +692,9 @@ exports.expectsError = function expectsError(fn, settings, exact) {
   }
   function innerFn(error) {
     assert.strictEqual(error.code, settings.code);
+    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)) {
diff --git a/test/parallel/test-dns-lookup.js b/test/parallel/test-dns-lookup.js
index a1fad873e33f37..3f0017c36b56b4 100644
--- a/test/parallel/test-dns-lookup.js
+++ b/test/parallel/test-dns-lookup.js
@@ -81,6 +81,9 @@ assert.doesNotThrow(() => {
     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
diff --git a/test/parallel/test-dns-resolveany-bad-ancount.js b/test/parallel/test-dns-resolveany-bad-ancount.js
index 25ce15935caad9..63ed1774b11933 100644
--- a/test/parallel/test-dns-resolveany-bad-ancount.js
+++ b/test/parallel/test-dns-resolveany-bad-ancount.js
@@ -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();
   }));
 }));