child_process: improve killSignal validations

As it is, the `killSignal` is just retrieved from an object and used.
If the signal passed is actually one of the inherited properties of
that object, Node.js will die. For example,

    ➜  node -e "child_process.spawnSync('ls', {killSignal: 'toString'})"
    Assertion failed: (0), function uv_close, file ....core.c, line 166.
    [1]    58938 abort      node -e "child_process.spawnSync(...)"

1. This patch makes sure that the signal is actually a own property of
   the constants object.

2. Extends the killSignal validation to all the other functions.

PR-URL: https://github.com/nodejs/node/pull/10423

Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>

1 files changed, 18 insertions, 3 deletions

diff --git a/test/parallel/test-child-process-spawnsync-validation-errors.js b/test/parallel/test-child-process-spawnsync-validation-errors.js
index c49e4960b0..83b114f7ba 100644
--- a/test/parallel/test-child-process-spawnsync-validation-errors.js
+++ b/test/parallel/test-child-process-spawnsync-validation-errors.js
@@ -2,6 +2,7 @@
 const common = require('../common');
 const assert = require('assert');
 const spawnSync = require('child_process').spawnSync;
+const signals = process.binding('constants').os.signals;
 
 function pass(option, value) {
   // Run the command with the specified option. Since it's not a real command,
@@ -184,18 +185,32 @@ if (!common.isWindows) {
 {
   // Validate the killSignal option
   const typeErr = /^TypeError: "killSignal" must be a string or number$/;
-  const rangeErr = /^RangeError: "killSignal" cannot be 0$/;
   const unknownSignalErr = /^Error: Unknown signal:/;
 
   pass('killSignal', undefined);
   pass('killSignal', null);
   pass('killSignal', 'SIGKILL');
-  pass('killSignal', 500);
-  fail('killSignal', 0, rangeErr);
   fail('killSignal', 'SIGNOTAVALIDSIGNALNAME', unknownSignalErr);
   fail('killSignal', true, typeErr);
   fail('killSignal', false, typeErr);
   fail('killSignal', [], typeErr);
   fail('killSignal', {}, typeErr);
   fail('killSignal', common.noop, typeErr);
+
+  // Invalid signal names and numbers should fail
+  fail('killSignal', 500, unknownSignalErr);
+  fail('killSignal', 0, unknownSignalErr);
+  fail('killSignal', -200, unknownSignalErr);
+  fail('killSignal', 3.14, unknownSignalErr);
+
+  Object.getOwnPropertyNames(Object.prototype).forEach((property) => {
+    fail('killSignal', property, unknownSignalErr);
+  });
+
+  // Valid signal names and numbers should pass
+  for (const signalName in signals) {
+    pass('killSignal', signals[signalName]);
+    pass('killSignal', signalName);
+    pass('killSignal', signalName.toLowerCase());
+  }
 }