René's URL Explorer Experiment

{"@context":"https://schema.org","@type":"DiscussionForumPosting","headline":"Inconsistent validation of custom signals","articleBody":"### Version\r\n\r\n18.9.0\r\n\r\n### Platform\r\n\r\nDarwin\r\n\r\n### Subsystem\r\n\r\nchild_process\r\nos\r\n\r\n### What steps will reproduce the bug?\r\n\r\nIn the current implementation, `spawn` and related functions throw synchronous errors for invalid signals, e.g.:\r\n\r\n```js\r\nimport { spawn } from 'node:child_process';\r\n\r\n// TypeError [ERR_UNKNOWN_SIGNAL]\r\nspawn(process.execPath, ['-v'], { killSignal: 12345 });\r\n```\r\n\r\nAnyway, it's easy to define custom signals that pass the validation:\r\n\r\n```js\r\nimport { spawn } from 'node:child_process';\r\nimport { constants } from 'node:os';\r\n\r\nconstants.signals.FOOBAR = 12345;\r\n// no error\r\nspawn(process.execPath, ['-v'], { killSignal: 12345 });\r\n```\r\n\r\nThe same applies when signals are identified by name, i.e:\r\n\r\n```js\r\nimport { spawn } from 'node:child_process';\r\n\r\n// TypeError [ERR_UNKNOWN_SIGNAL]\r\nspawn(process.execPath, ['-v'], { killSignal: 'FOOBAR' });\r\n```\r\n\r\nand\r\n\r\n```js\r\nimport { spawn } from 'node:child_process';\r\nimport { constants } from 'node:os';\r\n\r\nconstants.signals.FOOBAR = 12345;\r\n// no error\r\nspawn(process.execPath, ['-v'], { killSignal: 'FOOBAR' });\r\n```\r\n\r\n**but** if a custom signal is added after the internal mapping of numeric codes to names signals has been generated, validation fails for numeric codes (but not for signal names):\r\n\r\n```js\r\nimport { spawn } from 'node:child_process';\r\nimport { constants } from 'node:os';\r\n\r\nspawn(process.execPath, ['-v'], { killSignal: 1 });\r\nconstants.signals.FOOBAR = 12345;\r\nspawn(process.execPath, ['-v'], { killSignal: 'FOOBAR' }); // no error\r\nspawn(process.execPath, ['-v'], { killSignal: 12345 }); // TypeError [ERR_UNKNOWN_SIGNAL]\r\n```\r\n\r\nThis strikes to me as issue of improper memoization:\r\n\r\nhttps://github.com/nodejs/node/blob/9ae2af4aaebe5c6efad774989c4958ba6d32c0fd/lib/internal/util.js#L272-L283\r\n\r\nwhere `signalsToNamesMapping` never changes once created.\r\n\r\nOn the other hand, I'm not convinced that the current behavior - being able to add, remove, and change signals at will - is intended. If it is, then the validation logic should be fixed. If it is not, then `constants.signals` should be better frozen to prevent changes.\r\n\r\n### How often does it reproduce? Is there a required condition?\r\n\r\nAll the time.\r\n\r\n### What is the expected behavior?\r\n\r\nEither custom numeric signals should always be accepted as valid, or they should always be rejected. If they should be rejected, then custom string signals should be rejected, too.\r\n\r\n### What do you see instead?\r\n\r\nInconsistent behavior, depending on (possibly unrelated) previous operations.\r\n\r\n### Additional information\r\n\r\n_No response_","author":{"url":"https://github.com/fasttime","@type":"Person","name":"fasttime"},"datePublished":"2022-09-22T17:50:07.000Z","interactionStatistic":{"@type":"InteractionCounter","interactionType":"https://schema.org/CommentAction","userInteractionCount":4},"url":"https://github.com/44749/node/issues/44749"}