From 844755ab77199b6e222a13d1e8e4c67efb24a6ea Mon Sep 17 00:00:00 2001
From: Joyee Cheung <joyeec9h3@gmail.com>
Date: Wed, 17 Jan 2018 03:21:16 +0800
Subject: [PATCH] util: implement util.getSystemErrorName()

Reimplement uv.errname() as internal/util.getSystemErrorName() to
avoid the memory leaks caused by unknown error codes
and avoid calling into C++ for the error names. Also
expose it as a public API for external use.

Backport-PR-URL: https://github.com/nodejs/node/pull/19191
PR-URL: https://github.com/nodejs/node/pull/18186
Refs: http://docs.libuv.org/en/v1.x/errors.html#c.uv_err_name
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
---
 doc/api/util.md                               | 20 +++++++++++
 lib/child_process.js                          |  7 ++--
 lib/internal/util.js                          | 13 ++++++-
 lib/util.js                                   |  6 ++--
 src/uv.cc                                     |  2 ++
 test/parallel/test-child-process-execfile.js  |  5 +--
 .../parallel/test-net-server-listen-handle.js |  7 ++--
 test/parallel/test-uv-errno.js                | 35 +++++++++++++++++++
 test/sequential/test-async-wrap-getasyncid.js |  3 +-
 9 files changed, 85 insertions(+), 13 deletions(-)
 create mode 100644 test/parallel/test-uv-errno.js

diff --git a/doc/api/util.md b/doc/api/util.md
index 7932b815e4a5e9..490895c000a507 100644
--- a/doc/api/util.md
+++ b/doc/api/util.md
@@ -217,6 +217,25 @@ without any formatting.
 util.format('%% %s'); // '%% %s'
 ```
 
+## util.getSystemErrorName(err)
+<!-- YAML
+added: REPLACEME
+-->
+
+* `err` {number}
+* Returns: {string}
+
+Returns the string name for a numeric error code that comes from a Node.js API.
+The mapping between error codes and error names is platform-dependent.
+See [Common System Errors][] for the names of common errors.
+
+```js
+fs.access('file/that/does/not/exist', (err) => {
+  const name = util.getSystemErrorName(err.errno);
+  console.error(name);  // ENOENT
+});
+```
+
 ## util.inherits(constructor, superConstructor)
 <!-- YAML
 added: v0.3.0
@@ -1196,6 +1215,7 @@ Deprecated predecessor of `console.log`.
 [`console.log()`]: console.html#console_console_log_data_args
 [`util.inspect()`]: #util_util_inspect_object_options
 [`util.promisify()`]: #util_util_promisify_original
+[Common System Errors]: errors.html#errors_common_system_errors
 [Custom inspection functions on Objects]: #util_custom_inspection_functions_on_objects
 [Custom promisified functions]: #util_custom_promisified_functions
 [Customizing `util.inspect` colors]: #util_customizing_util_inspect_colors
diff --git a/lib/child_process.js b/lib/child_process.js
index 2e6360d42b9246..aa24c9f44ea0ab 100644
--- a/lib/child_process.js
+++ b/lib/child_process.js
@@ -22,13 +22,14 @@
 'use strict';
 
 const util = require('util');
-const { deprecate, convertToValidSignal } = require('internal/util');
+const {
+  deprecate, convertToValidSignal, getSystemErrorName
+} = require('internal/util');
 const { isUint8Array } = require('internal/util/types');
 const { createPromise,
         promiseResolve, promiseReject } = process.binding('util');
 const debug = util.debuglog('child_process');
 
-const uv = process.binding('uv');
 const spawn_sync = process.binding('spawn_sync');
 const { Buffer } = require('buffer');
 const { Pipe, constants: PipeConstants } = process.binding('pipe_wrap');
@@ -274,7 +275,7 @@ exports.execFile = function(file /*, args, options, callback*/) {
     if (!ex) {
       ex = new Error('Command failed: ' + cmd + '\n' + stderr);
       ex.killed = child.killed || killed;
-      ex.code = code < 0 ? uv.errname(code) : code;
+      ex.code = code < 0 ? getSystemErrorName(code) : code;
       ex.signal = signal;
     }
 
diff --git a/lib/internal/util.js b/lib/internal/util.js
index 8fc040d06714dc..d915a699b54717 100644
--- a/lib/internal/util.js
+++ b/lib/internal/util.js
@@ -3,8 +3,8 @@
 const errors = require('internal/errors');
 const binding = process.binding('util');
 const { signals } = process.binding('constants').os;
-
 const { createPromise, promiseResolve, promiseReject } = binding;
+const { errmap } = process.binding('uv');
 
 const kArrowMessagePrivateSymbolIndex = binding.arrow_message_private_symbol;
 const kDecoratedPrivateSymbolIndex = binding.decorated_private_symbol;
@@ -201,6 +201,16 @@ function getConstructorOf(obj) {
   return null;
 }
 
+function getSystemErrorName(err) {
+  if (typeof err !== 'number' || err >= 0 || !Number.isSafeInteger(err)) {
+    throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'err',
+                               'negative number');
+  }
+
+  const entry = errmap.get(err);
+  return entry ? entry[0] : `Unknown system error ${err}`;
+}
+
 const kCustomPromisifiedSymbol = Symbol('util.promisify.custom');
 const kCustomPromisifyArgsSymbol = Symbol('customPromisifyArgs');
 
@@ -286,6 +296,7 @@ module.exports = {
   emitExperimentalWarning,
   filterDuplicateStrings,
   getConstructorOf,
+  getSystemErrorName,
   isError,
   join,
   normalizeEncoding,
diff --git a/lib/util.js b/lib/util.js
index 4c6db1a479e7b3..45f580d08f4d72 100644
--- a/lib/util.js
+++ b/lib/util.js
@@ -25,8 +25,6 @@ const errors = require('internal/errors');
 const { TextDecoder, TextEncoder } = require('internal/encoding');
 const { isBuffer } = require('buffer').Buffer;
 
-const { errname } = process.binding('uv');
-
 const {
   getPromiseDetails,
   getProxyDetails,
@@ -52,6 +50,7 @@ const {
   customInspectSymbol,
   deprecate,
   getConstructorOf,
+  getSystemErrorName,
   isError,
   promisify,
   join
@@ -985,7 +984,7 @@ function error(...args) {
 }
 
 function _errnoException(err, syscall, original) {
-  var name = errname(err);
+  const name = getSystemErrorName(err);
   var message = `${syscall} ${name}`;
   if (original)
     message += ` ${original}`;
@@ -1075,6 +1074,7 @@ module.exports = exports = {
   debuglog,
   deprecate,
   format,
+  getSystemErrorName,
   inherits,
   inspect,
   isArray: Array.isArray,
diff --git a/src/uv.cc b/src/uv.cc
index 5ad53a0024b1ea..db650377d8092d 100644
--- a/src/uv.cc
+++ b/src/uv.cc
@@ -39,6 +39,8 @@ using v8::String;
 using v8::Value;
 
 
+// TODO(joyeecheung): deprecate this function in favor of
+// lib/util.getSystemErrorName()
 void ErrName(const FunctionCallbackInfo<Value>& args) {
   Environment* env = Environment::GetCurrent(args);
   int err = args[0]->Int32Value();
diff --git a/test/parallel/test-child-process-execfile.js b/test/parallel/test-child-process-execfile.js
index a64128d6a3ab6b..af69d685789d62 100644
--- a/test/parallel/test-child-process-execfile.js
+++ b/test/parallel/test-child-process-execfile.js
@@ -1,8 +1,9 @@
 'use strict';
+
 const common = require('../common');
 const assert = require('assert');
 const execFile = require('child_process').execFile;
-const uv = process.binding('uv');
+const { getSystemErrorName } = require('util');
 const fixtures = require('../common/fixtures');
 
 const fixture = fixtures.path('exit.js');
@@ -27,7 +28,7 @@ const execOpts = { encoding: 'utf8', shell: true };
   const code = -1;
   const callback = common.mustCall((err, stdout, stderr) => {
     assert.strictEqual(err.toString().trim(), errorString);
-    assert.strictEqual(err.code, uv.errname(code));
+    assert.strictEqual(err.code, getSystemErrorName(code));
     assert.strictEqual(err.killed, true);
     assert.strictEqual(err.signal, null);
     assert.strictEqual(err.cmd, process.execPath);
diff --git a/test/parallel/test-net-server-listen-handle.js b/test/parallel/test-net-server-listen-handle.js
index 532674bc7a2819..5bf8451302fae9 100644
--- a/test/parallel/test-net-server-listen-handle.js
+++ b/test/parallel/test-net-server-listen-handle.js
@@ -4,7 +4,7 @@ const common = require('../common');
 const assert = require('assert');
 const net = require('net');
 const fs = require('fs');
-const uv = process.binding('uv');
+const { getSystemErrorName } = require('util');
 const { TCP, constants: TCPConstants } = process.binding('tcp_wrap');
 const { Pipe, constants: PipeConstants } = process.binding('pipe_wrap');
 
@@ -47,9 +47,10 @@ function randomHandle(type) {
     handleName = `pipe ${path}`;
   }
 
-  if (errno < 0) {  // uv.errname requires err < 0
-    assert(errno >= 0, `unable to bind ${handleName}: ${uv.errname(errno)}`);
+  if (errno < 0) {
+    assert.fail(`unable to bind ${handleName}: ${getSystemErrorName(errno)}`);
   }
+
   if (!common.isWindows) {  // fd doesn't work on windows
     // err >= 0 but fd = -1, should not happen
     assert.notStrictEqual(handle.fd, -1,
diff --git a/test/parallel/test-uv-errno.js b/test/parallel/test-uv-errno.js
new file mode 100644
index 00000000000000..28f43db94a247d
--- /dev/null
+++ b/test/parallel/test-uv-errno.js
@@ -0,0 +1,35 @@
+'use strict';
+
+const common = require('../common');
+const assert = require('assert');
+const {
+  getSystemErrorName,
+  _errnoException
+} = require('util');
+
+const uv = process.binding('uv');
+const keys = Object.keys(uv);
+
+keys.forEach((key) => {
+  if (!key.startsWith('UV_'))
+    return;
+
+  assert.doesNotThrow(() => {
+    const err = _errnoException(uv[key], 'test');
+    const name = uv.errname(uv[key]);
+    assert.strictEqual(getSystemErrorName(uv[key]), name);
+    assert.strictEqual(err.code, name);
+    assert.strictEqual(err.code, err.errno);
+    assert.strictEqual(err.message, `test ${name}`);
+  });
+});
+
+[0, 1, 'test', {}, [], Infinity, -Infinity, NaN].forEach((key) => {
+  common.expectsError(
+    () => _errnoException(key),
+    {
+      code: 'ERR_INVALID_ARG_TYPE',
+      type: TypeError,
+      message: 'The "err" argument must be of type negative number'
+    });
+});
diff --git a/test/sequential/test-async-wrap-getasyncid.js b/test/sequential/test-async-wrap-getasyncid.js
index c8c25725957824..16e235b97e2119 100644
--- a/test/sequential/test-async-wrap-getasyncid.js
+++ b/test/sequential/test-async-wrap-getasyncid.js
@@ -7,6 +7,7 @@ const net = require('net');
 const providers = Object.assign({}, process.binding('async_wrap').Providers);
 const fixtures = require('../common/fixtures');
 const tmpdir = require('../common/tmpdir');
+const { getSystemErrorName } = require('util');
 
 // Make sure that all Providers are tested.
 {
@@ -214,7 +215,7 @@ if (common.hasCrypto) { // eslint-disable-line crypto-check
       // Use a long string to make sure the write happens asynchronously.
       const err = handle.writeLatin1String(wreq, 'hi'.repeat(100000));
       if (err)
-        throw new Error(`write failed: ${process.binding('uv').errname(err)}`);
+        throw new Error(`write failed: ${getSystemErrorName(err)}`);
       testInitialized(wreq, 'WriteWrap');
     });
     req.address = common.localhostIPv4;