From 23a70c0bac434fe96391412563d875589a96fd2c Mon Sep 17 00:00:00 2001 From: RedYetiDev <38299977+RedYetiDev@users.noreply.github.com> Date: Mon, 9 Sep 2024 19:10:38 -0400 Subject: [PATCH 1/6] repl: runtime deprecate instantiating without new --- doc/api/deprecations.md | 5 ++++- lib/internal/util.js | 18 ++++++++++++------ lib/repl.js | 10 ++++------ .../test-repl-preview-without-inspector.js | 2 +- test/parallel/test-repl-preview.js | 2 +- test/parallel/test-repl.js | 10 ++++++++++ 6 files changed, 32 insertions(+), 15 deletions(-) diff --git a/doc/api/deprecations.md b/doc/api/deprecations.md index 9c5ded0dd3b9dc..0d62af9ba608b6 100644 --- a/doc/api/deprecations.md +++ b/doc/api/deprecations.md @@ -3748,6 +3748,9 @@ It is recommended to use the `new` qualifier instead. This applies to all Zlib c -Type: Documentation-only +Type: Runtime Instantiating classes without the `new` qualifier exported by the `node:repl` module is deprecated. It is recommended to use the `new` qualifier instead. This applies to all REPL classes, including diff --git a/lib/internal/util.js b/lib/internal/util.js index 4d8cf99a3453d9..6ca3239e6f9ffc 100644 --- a/lib/internal/util.js +++ b/lib/internal/util.js @@ -97,7 +97,7 @@ function isError(e) { // each one once. const codesWarned = new SafeSet(); -let validateString; +const lazyValidateString = getLazy(() => require('internal/validators').validateString); function getDeprecationWarningEmitter( code, msg, deprecated, useEmitSync, @@ -145,12 +145,8 @@ function pendingDeprecate(fn, msg, code) { // Returns a modified function which warns once by default. // If --no-deprecation is set, then it is a no-op. function deprecate(fn, msg, code, useEmitSync) { - // Lazy-load to avoid a circular dependency. - if (validateString === undefined) - ({ validateString } = require('internal/validators')); - if (code !== undefined) - validateString(code, 'code'); + lazyValidateString()(code, 'code'); const emitDeprecationWarning = getDeprecationWarningEmitter( code, msg, deprecated, useEmitSync, @@ -179,6 +175,15 @@ function deprecate(fn, msg, code, useEmitSync) { return deprecated; } +function deprecateInstantation(target, code, ...args) { + if (code !== undefined) + lazyValidateString()(code, 'code'); + + getDeprecationWarningEmitter(code, `Instantiating ${target.name} without the 'new' keyword has been deprecated.`, target)(); + + return ReflectConstruct(target, args); +} + function decorateErrorStack(err) { if (!(isError(err) && err.stack) || err[decorated_private_symbol]) return; @@ -873,6 +878,7 @@ module.exports = { defineLazyProperties, defineReplaceableLazyAttribute, deprecate, + deprecateInstantation, emitExperimentalWarning, encodingsMap, exposeInterface, diff --git a/lib/repl.js b/lib/repl.js index 37b34af2917643..871ea680b221df 100644 --- a/lib/repl.js +++ b/lib/repl.js @@ -111,6 +111,7 @@ const { decorateErrorStack, isError, deprecate, + deprecateInstantation, SideEffectFreeRegExpPrototypeSymbolReplace, SideEffectFreeRegExpPrototypeSymbolSplit, } = require('internal/util'); @@ -262,12 +263,7 @@ function REPLServer(prompt, ignoreUndefined, replMode) { if (!(this instanceof REPLServer)) { - return new REPLServer(prompt, - stream, - eval_, - useGlobal, - ignoreUndefined, - replMode); + return deprecateInstantation(REPLServer, 'DEP0185', prompt, stream, eval_, useGlobal, ignoreUndefined, replMode); } let options; @@ -1848,6 +1844,8 @@ function defineDefaultCommands(repl) { } function Recoverable(err) { + if (!(this instanceof Recoverable)) + return deprecateInstantation(Recoverable, 'DEP0185', err); this.err = err; } ObjectSetPrototypeOf(Recoverable.prototype, SyntaxErrorPrototype); diff --git a/test/parallel/test-repl-preview-without-inspector.js b/test/parallel/test-repl-preview-without-inspector.js index 8905d214836c64..67090a928feba4 100644 --- a/test/parallel/test-repl-preview-without-inspector.js +++ b/test/parallel/test-repl-preview-without-inspector.js @@ -65,7 +65,7 @@ function runAndWait(cmds, repl) { return promise; } -const repl = REPLServer({ +const repl = new REPLServer({ prompt: PROMPT, stream: new REPLStream(), ignoreUndefined: true, diff --git a/test/parallel/test-repl-preview.js b/test/parallel/test-repl-preview.js index 6eb2a169918a51..06a044f5a11a13 100644 --- a/test/parallel/test-repl-preview.js +++ b/test/parallel/test-repl-preview.js @@ -57,7 +57,7 @@ function runAndWait(cmds, repl) { } async function tests(options) { - const repl = REPLServer({ + const repl = new REPLServer({ prompt: PROMPT, stream: new REPLStream(), ignoreUndefined: true, diff --git a/test/parallel/test-repl.js b/test/parallel/test-repl.js index 610c7813e0439c..394d001b41d158 100644 --- a/test/parallel/test-repl.js +++ b/test/parallel/test-repl.js @@ -1035,3 +1035,13 @@ function event(ee, expected) { })); }); } + +{ + const server = repl.REPLServer(); + common.expectWarning({ + DeprecationWarning: { + DEP0185: 'Instantiating REPLServer without the \'new\' keyword has been deprecated.', + } + }); + server.emit('line', '.exit'); +} From e9a2c30526c9159f6c6eaa7d9b51eb000d3c6bc6 Mon Sep 17 00:00:00 2001 From: Aviv Keller Date: Sat, 26 Oct 2024 16:06:40 -0400 Subject: [PATCH 2/6] fixup! --- lib/internal/util.js | 18 +++++++++++------- lib/repl.js | 6 +++--- 2 files changed, 14 insertions(+), 10 deletions(-) diff --git a/lib/internal/util.js b/lib/internal/util.js index 6ca3239e6f9ffc..2df4695c9f1abc 100644 --- a/lib/internal/util.js +++ b/lib/internal/util.js @@ -63,6 +63,7 @@ const { } = internalBinding('util'); const { isNativeError, isPromise } = internalBinding('types'); const { getOptionValue } = require('internal/options'); +const assert = require('internal/assert'); const { encodings } = internalBinding('string_decoder'); const noCrypto = !process.versions.openssl; @@ -97,7 +98,7 @@ function isError(e) { // each one once. const codesWarned = new SafeSet(); -const lazyValidateString = getLazy(() => require('internal/validators').validateString); +let validateString; function getDeprecationWarningEmitter( code, msg, deprecated, useEmitSync, @@ -145,8 +146,12 @@ function pendingDeprecate(fn, msg, code) { // Returns a modified function which warns once by default. // If --no-deprecation is set, then it is a no-op. function deprecate(fn, msg, code, useEmitSync) { + // Lazy-load to avoid a circular dependency. + if (validateString === undefined) + ({ validateString } = require('internal/validators')); + if (code !== undefined) - lazyValidateString()(code, 'code'); + validateString(code, 'code'); const emitDeprecationWarning = getDeprecationWarningEmitter( code, msg, deprecated, useEmitSync, @@ -175,9 +180,8 @@ function deprecate(fn, msg, code, useEmitSync) { return deprecated; } -function deprecateInstantation(target, code, ...args) { - if (code !== undefined) - lazyValidateString()(code, 'code'); +function deprecateInstantiation(target, code, ...args) { + assert(typeof code === 'string'); getDeprecationWarningEmitter(code, `Instantiating ${target.name} without the 'new' keyword has been deprecated.`, target)(); @@ -878,7 +882,7 @@ module.exports = { defineLazyProperties, defineReplaceableLazyAttribute, deprecate, - deprecateInstantation, + deprecateInstantiation, emitExperimentalWarning, encodingsMap, exposeInterface, @@ -930,4 +934,4 @@ module.exports = { setOwnProperty, pendingDeprecate, WeakReference, -}; +}; \ No newline at end of file diff --git a/lib/repl.js b/lib/repl.js index 871ea680b221df..724a01bfc430af 100644 --- a/lib/repl.js +++ b/lib/repl.js @@ -111,7 +111,7 @@ const { decorateErrorStack, isError, deprecate, - deprecateInstantation, + deprecateInstantiation, SideEffectFreeRegExpPrototypeSymbolReplace, SideEffectFreeRegExpPrototypeSymbolSplit, } = require('internal/util'); @@ -263,7 +263,7 @@ function REPLServer(prompt, ignoreUndefined, replMode) { if (!(this instanceof REPLServer)) { - return deprecateInstantation(REPLServer, 'DEP0185', prompt, stream, eval_, useGlobal, ignoreUndefined, replMode); + return deprecateInstantiation(REPLServer, 'DEP0185', prompt, stream, eval_, useGlobal, ignoreUndefined, replMode); } let options; @@ -1845,7 +1845,7 @@ function defineDefaultCommands(repl) { function Recoverable(err) { if (!(this instanceof Recoverable)) - return deprecateInstantation(Recoverable, 'DEP0185', err); + return deprecateInstantiation(Recoverable, 'DEP0185', err); this.err = err; } ObjectSetPrototypeOf(Recoverable.prototype, SyntaxErrorPrototype); From 391f6e934b357df4b6339c119601276296d672cc Mon Sep 17 00:00:00 2001 From: RedYetiDev <38299977+RedYetiDev@users.noreply.github.com> Date: Sat, 26 Oct 2024 16:11:02 -0400 Subject: [PATCH 3/6] fixup! fixup! --- lib/internal/util.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/internal/util.js b/lib/internal/util.js index 2df4695c9f1abc..bb25da74b65d3d 100644 --- a/lib/internal/util.js +++ b/lib/internal/util.js @@ -934,4 +934,4 @@ module.exports = { setOwnProperty, pendingDeprecate, WeakReference, -}; \ No newline at end of file +}; From 547568cffb3278d2acb7c33e5e19c0214a793a69 Mon Sep 17 00:00:00 2001 From: Aviv Keller Date: Sat, 26 Oct 2024 16:29:58 -0400 Subject: [PATCH 4/6] fixup! fixup! fixup! Co-authored-by: Antoine du Hamel --- lib/repl.js | 2 -- 1 file changed, 2 deletions(-) diff --git a/lib/repl.js b/lib/repl.js index 724a01bfc430af..c195ba428ce6c2 100644 --- a/lib/repl.js +++ b/lib/repl.js @@ -1844,8 +1844,6 @@ function defineDefaultCommands(repl) { } function Recoverable(err) { - if (!(this instanceof Recoverable)) - return deprecateInstantiation(Recoverable, 'DEP0185', err); this.err = err; } ObjectSetPrototypeOf(Recoverable.prototype, SyntaxErrorPrototype); From 1974e39fa6e05959738dc19139fd38927e365470 Mon Sep 17 00:00:00 2001 From: RedYetiDev <38299977+RedYetiDev@users.noreply.github.com> Date: Mon, 28 Oct 2024 16:50:39 -0400 Subject: [PATCH 5/6] fix flaky test --- test/parallel/test-repl.js | 1 + 1 file changed, 1 insertion(+) diff --git a/test/parallel/test-repl.js b/test/parallel/test-repl.js index 394d001b41d158..8520bf15d01af6 100644 --- a/test/parallel/test-repl.js +++ b/test/parallel/test-repl.js @@ -1041,6 +1041,7 @@ function event(ee, expected) { common.expectWarning({ DeprecationWarning: { DEP0185: 'Instantiating REPLServer without the \'new\' keyword has been deprecated.', + DEP0169: '`url.parse()` behavior is not standardized and prone to errors that have security implications. Use the WHATWG URL API instead. CVEs are not issued for `url.parse()` vulnerabilities.', } }); server.emit('line', '.exit'); From 9bf85bc41c0686e5516420997cf384d5887aa53f Mon Sep 17 00:00:00 2001 From: RedYetiDev <38299977+RedYetiDev@users.noreply.github.com> Date: Mon, 28 Oct 2024 16:59:32 -0400 Subject: [PATCH 6/6] fixup! fix flaky test --- test/parallel/test-repl.js | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/test/parallel/test-repl.js b/test/parallel/test-repl.js index 8520bf15d01af6..be51efd2143d31 100644 --- a/test/parallel/test-repl.js +++ b/test/parallel/test-repl.js @@ -1041,7 +1041,10 @@ function event(ee, expected) { common.expectWarning({ DeprecationWarning: { DEP0185: 'Instantiating REPLServer without the \'new\' keyword has been deprecated.', - DEP0169: '`url.parse()` behavior is not standardized and prone to errors that have security implications. Use the WHATWG URL API instead. CVEs are not issued for `url.parse()` vulnerabilities.', + // For the 'url.format' test-case. + DEP0169: + '`url.parse()` behavior is not standardized and prone to errors that have security implications. ' + + 'Use the WHATWG URL API instead. CVEs are not issued for `url.parse()` vulnerabilities.', } }); server.emit('line', '.exit');