From 8253a307ce405a1cd5c35245abc7f6e47f9c16bc Mon Sep 17 00:00:00 2001 From: Daeyeon Jeong Date: Sat, 14 May 2022 22:44:26 +0900 Subject: [PATCH 1/4] console: fix console.dir crash on a revoked proxy Fixes: https://github.com/nodejs/node/issues/43095 Signed-off-by: Daeyeon Jeong daeyeon.dev@gmail.com --- lib/internal/util/inspect.js | 25 ++++++++++++++--------- test/parallel/test-console-issue-43095.js | 12 +++++++++++ test/parallel/test-util-inspect-proxy.js | 19 +++++++++++++++++ 3 files changed, 46 insertions(+), 10 deletions(-) create mode 100644 test/parallel/test-console-issue-43095.js diff --git a/lib/internal/util/inspect.js b/lib/internal/util/inspect.js index d78a4e97d218a6..a28a561f179f03 100644 --- a/lib/internal/util/inspect.js +++ b/lib/internal/util/inspect.js @@ -724,16 +724,18 @@ function getCtxStyle(value, constructor, tag) { return getPrefix(constructor, tag, fallback); } -function formatProxy(ctx, proxy, recurseTimes) { +function formatProxy(ctx, target, handler, recurseTimes, showProperties = true) { if (recurseTimes > ctx.depth && ctx.depth !== null) { return ctx.stylize('Proxy [Array]', 'special'); } recurseTimes += 1; ctx.indentationLvl += 2; - const res = [ - formatValue(ctx, proxy[0], recurseTimes), - formatValue(ctx, proxy[1], recurseTimes), - ]; + const res = showProperties ? + [ + formatValue(ctx, target, recurseTimes), + formatValue(ctx, handler, recurseTimes), + ] : + []; ctx.indentationLvl -= 2; return reduceToSingleString( ctx, res, '', ['Proxy [', ']'], kArrayExtrasType, recurseTimes); @@ -757,12 +759,15 @@ function formatValue(ctx, value, recurseTimes, typedArray) { const context = value; // Always check for proxies to prevent side effects and to prevent triggering // any proxy handlers. - const proxy = getProxyDetails(value, !!ctx.showProxy); - if (proxy !== undefined) { + const details = getProxyDetails(value, !!ctx.showProxy); + if (details !== undefined) { if (ctx.showProxy) { - return formatProxy(ctx, proxy, recurseTimes); + return formatProxy(ctx, details[0], details[1], recurseTimes); + } else if (details === null) { + // The proxy is revoked. Both target and handler of it are null. + return formatProxy(ctx, details, null, recurseTimes, ctx.showProxy); } - value = proxy; + value = details; } // Provide a hook for user-specified inspect functions. @@ -778,7 +783,7 @@ function formatValue(ctx, value, recurseTimes, typedArray) { // a counter internally. const depth = ctx.depth === null ? null : ctx.depth - recurseTimes; const isCrossContext = - proxy !== undefined || !(context instanceof Object); + details !== undefined || !(context instanceof Object); const ret = FunctionPrototypeCall( maybeCustom, context, diff --git a/test/parallel/test-console-issue-43095.js b/test/parallel/test-console-issue-43095.js new file mode 100644 index 00000000000000..647f4af2df4f96 --- /dev/null +++ b/test/parallel/test-console-issue-43095.js @@ -0,0 +1,12 @@ +'use strict'; + +require('../common'); +const { inspect } = require('node:util'); + +const r = Proxy.revocable({}, {}); +r.revoke(); + +console.dir(r); +console.dir(r.proxy); +console.log(r.proxy); +console.log(inspect(r.proxy, { showProxy: true })); diff --git a/test/parallel/test-util-inspect-proxy.js b/test/parallel/test-util-inspect-proxy.js index 3e1341fadfc9f9..fe69409a63a980 100644 --- a/test/parallel/test-util-inspect-proxy.js +++ b/test/parallel/test-util-inspect-proxy.js @@ -57,6 +57,25 @@ assert.strictEqual(handler, details[1]); details = processUtil.getProxyDetails(proxyObj, false); assert.strictEqual(target, details); +details = processUtil.getProxyDetails({}, true); +assert.strictEqual(details, undefined); + +const r = Proxy.revocable({}, {}); +r.revoke(); + +details = processUtil.getProxyDetails(r.proxy, true); +assert.strictEqual(details[0], null); +assert.strictEqual(details[1], null); + +details = processUtil.getProxyDetails(r.proxy, false); +assert.strictEqual(details, null); + +assert.strictEqual(util.inspect(r.proxy), 'Proxy [ ]'); +assert.strictEqual( + util.inspect(r, { showProxy: true }), + '{ proxy: Proxy [ null, null ], revoke: [Function (anonymous)] }', +); + assert.strictEqual( util.inspect(proxyObj, opts), 'Proxy [\n' + From 5723aaf3a3460120e6f20039836c033024036076 Mon Sep 17 00:00:00 2001 From: Daeyeon Jeong Date: Sun, 15 May 2022 03:51:49 +0900 Subject: [PATCH 2/4] console: remove spaces for the output is empty This commit removes extra spaces looking unnecessary if the `joinedOutput` of type `Array` is empty on `reduceToSingleString`. e.g) Proxy [ ] -> Proxy [] Signed-off-by: Daeyeon Jeong daeyeon.dev@gmail.com --- lib/internal/util/inspect.js | 7 +++++-- test/parallel/test-util-inspect-proxy.js | 2 +- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/lib/internal/util/inspect.js b/lib/internal/util/inspect.js index a28a561f179f03..97ba51959d81cc 100644 --- a/lib/internal/util/inspect.js +++ b/lib/internal/util/inspect.js @@ -1939,9 +1939,12 @@ function reduceToSingleString( braces[0].length + base.length + 10; if (isBelowBreakLength(ctx, output, start, base)) { const joinedOutput = join(output, ', '); + const space = joinedOutput.length > 0 ? ' ' : ''; if (!joinedOutput.includes('\n')) { - return `${base ? `${base} ` : ''}${braces[0]} ${joinedOutput}` + - ` ${braces[1]}`; + return ( + `${base ? `${base} ` : ''}${braces[0]}${space}${joinedOutput}` + + `${space}${braces[1]}` + ); } } } diff --git a/test/parallel/test-util-inspect-proxy.js b/test/parallel/test-util-inspect-proxy.js index fe69409a63a980..45b8449aac2fac 100644 --- a/test/parallel/test-util-inspect-proxy.js +++ b/test/parallel/test-util-inspect-proxy.js @@ -70,7 +70,7 @@ assert.strictEqual(details[1], null); details = processUtil.getProxyDetails(r.proxy, false); assert.strictEqual(details, null); -assert.strictEqual(util.inspect(r.proxy), 'Proxy [ ]'); +assert.strictEqual(util.inspect(r.proxy), 'Proxy []'); assert.strictEqual( util.inspect(r, { showProxy: true }), '{ proxy: Proxy [ null, null ], revoke: [Function (anonymous)] }', From 161dbe50ca5cf6ae170c72166af706121dc3b5d7 Mon Sep 17 00:00:00 2001 From: Daeyeon Jeong Date: Sun, 15 May 2022 17:29:53 +0900 Subject: [PATCH 3/4] fixup: fix util.format on a revoked proxy Signed-off-by: Daeyeon Jeong daeyeon.dev@gmail.com --- lib/internal/util/inspect.js | 2 +- test/parallel/test-util-inspect-proxy.js | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/internal/util/inspect.js b/lib/internal/util/inspect.js index 97ba51959d81cc..0b1a776c9cd1bb 100644 --- a/lib/internal/util/inspect.js +++ b/lib/internal/util/inspect.js @@ -1979,7 +1979,7 @@ function hasBuiltInToString(value) { } // Count objects that have no `toString` function as built-in. - if (typeof value.toString !== 'function') { + if (typeof value?.toString !== 'function') { return true; } diff --git a/test/parallel/test-util-inspect-proxy.js b/test/parallel/test-util-inspect-proxy.js index 45b8449aac2fac..30365f43f7b9a6 100644 --- a/test/parallel/test-util-inspect-proxy.js +++ b/test/parallel/test-util-inspect-proxy.js @@ -76,6 +76,8 @@ assert.strictEqual( '{ proxy: Proxy [ null, null ], revoke: [Function (anonymous)] }', ); +assert.strictEqual(util.format('%s', r.proxy), 'Proxy []'); + assert.strictEqual( util.inspect(proxyObj, opts), 'Proxy [\n' + From 13964d0edd9f482db6b3633cfcbffab27fdb9406 Mon Sep 17 00:00:00 2001 From: Daeyeon Jeong Date: Thu, 19 May 2022 17:58:20 +0900 Subject: [PATCH 4/4] fixup: simplify the code and change visualization Signed-off-by: Daeyeon Jeong daeyeon.dev@gmail.com --- lib/internal/util/inspect.js | 40 +++++++++++------------- test/parallel/test-util-inspect-proxy.js | 6 ++-- 2 files changed, 22 insertions(+), 24 deletions(-) diff --git a/lib/internal/util/inspect.js b/lib/internal/util/inspect.js index 0b1a776c9cd1bb..7285014e803cd7 100644 --- a/lib/internal/util/inspect.js +++ b/lib/internal/util/inspect.js @@ -724,18 +724,16 @@ function getCtxStyle(value, constructor, tag) { return getPrefix(constructor, tag, fallback); } -function formatProxy(ctx, target, handler, recurseTimes, showProperties = true) { +function formatProxy(ctx, proxy, recurseTimes) { if (recurseTimes > ctx.depth && ctx.depth !== null) { return ctx.stylize('Proxy [Array]', 'special'); } recurseTimes += 1; ctx.indentationLvl += 2; - const res = showProperties ? - [ - formatValue(ctx, target, recurseTimes), - formatValue(ctx, handler, recurseTimes), - ] : - []; + const res = [ + formatValue(ctx, proxy[0], recurseTimes), + formatValue(ctx, proxy[1], recurseTimes), + ]; ctx.indentationLvl -= 2; return reduceToSingleString( ctx, res, '', ['Proxy [', ']'], kArrayExtrasType, recurseTimes); @@ -759,15 +757,15 @@ function formatValue(ctx, value, recurseTimes, typedArray) { const context = value; // Always check for proxies to prevent side effects and to prevent triggering // any proxy handlers. - const details = getProxyDetails(value, !!ctx.showProxy); - if (details !== undefined) { + const proxy = getProxyDetails(value, !!ctx.showProxy); + if (proxy !== undefined) { + if (proxy === null || proxy[0] === null) { + return ctx.stylize('', 'special'); + } if (ctx.showProxy) { - return formatProxy(ctx, details[0], details[1], recurseTimes); - } else if (details === null) { - // The proxy is revoked. Both target and handler of it are null. - return formatProxy(ctx, details, null, recurseTimes, ctx.showProxy); + return formatProxy(ctx, proxy, recurseTimes); } - value = details; + value = proxy; } // Provide a hook for user-specified inspect functions. @@ -783,7 +781,7 @@ function formatValue(ctx, value, recurseTimes, typedArray) { // a counter internally. const depth = ctx.depth === null ? null : ctx.depth - recurseTimes; const isCrossContext = - details !== undefined || !(context instanceof Object); + proxy !== undefined || !(context instanceof Object); const ret = FunctionPrototypeCall( maybeCustom, context, @@ -1939,12 +1937,9 @@ function reduceToSingleString( braces[0].length + base.length + 10; if (isBelowBreakLength(ctx, output, start, base)) { const joinedOutput = join(output, ', '); - const space = joinedOutput.length > 0 ? ' ' : ''; if (!joinedOutput.includes('\n')) { - return ( - `${base ? `${base} ` : ''}${braces[0]}${space}${joinedOutput}` + - `${space}${braces[1]}` - ); + return `${base ? `${base} ` : ''}${braces[0]} ${joinedOutput}` + + ` ${braces[1]}`; } } } @@ -1975,11 +1970,14 @@ function hasBuiltInToString(value) { const getFullProxy = false; const proxyTarget = getProxyDetails(value, getFullProxy); if (proxyTarget !== undefined) { + if (proxyTarget === null) { + return true; + } value = proxyTarget; } // Count objects that have no `toString` function as built-in. - if (typeof value?.toString !== 'function') { + if (typeof value.toString !== 'function') { return true; } diff --git a/test/parallel/test-util-inspect-proxy.js b/test/parallel/test-util-inspect-proxy.js index 30365f43f7b9a6..6344adae990860 100644 --- a/test/parallel/test-util-inspect-proxy.js +++ b/test/parallel/test-util-inspect-proxy.js @@ -70,13 +70,13 @@ assert.strictEqual(details[1], null); details = processUtil.getProxyDetails(r.proxy, false); assert.strictEqual(details, null); -assert.strictEqual(util.inspect(r.proxy), 'Proxy []'); +assert.strictEqual(util.inspect(r.proxy), ''); assert.strictEqual( util.inspect(r, { showProxy: true }), - '{ proxy: Proxy [ null, null ], revoke: [Function (anonymous)] }', + '{ proxy: , revoke: [Function (anonymous)] }', ); -assert.strictEqual(util.format('%s', r.proxy), 'Proxy []'); +assert.strictEqual(util.format('%s', r.proxy), ''); assert.strictEqual( util.inspect(proxyObj, opts),