Skip to content

Commit dd7dfd4

Browse files
committed
assert: surface prototype mismatch in deepStrictEqual diff
deepStrictEqual requires both values to share the same prototype, but the generated diff may not make that failure cause obvious: when both values are inspected identically (e.g. an instance of an anonymous class compared to a plain object) the structural diff shows no difference at all, and when a subclass is involved the mismatch is only visible implicitly through the inspected class-name prefix. Append an explicit "Object prototypes differ: X !== Y" line to the generated message when the operator is deepStrictEqual, both values are objects, their top-level prototypes differ, and at least one of the two prototypes is not a default prototype (Object.prototype, Array.prototype or null), because those cases are already clearly visible in the inspect output. The diagnostic covers the top-level values only; prototype differences of nested objects are not reported. The diagnostic is derived defensively: the prototype reads and the prototype names (derived from a single read of the constructor's name) are wrapped in a try/catch so that exotic objects (e.g. a Proxy with a throwing `getPrototypeOf` trap or a stateful `name` getter) cannot replace the assertion error with a different exception nor leave the global `Error.stackTraceLimit` modified; the hint is simply omitted in that case. When the comparison is made with `skipPrototype: true` (via `new assert.Assert({ skipPrototype: true })`), the hint is not added because the explicitly ignored difference is not the cause of the failure. Refs: #50397 Assisted-by: ZCode (GLM) Signed-off-by: vaputa <2475834+vaputa@users.noreply.github.com>
1 parent 2152942 commit dd7dfd4

6 files changed

Lines changed: 325 additions & 8 deletions

File tree

‎doc/api/assert.md‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -172,6 +172,9 @@ added: v0.1.21
172172
frames before this function.
173173
* `diff` {string} If set to `'full'`, shows the full diff in assertion errors. Defaults to `'simple'`.
174174
Accepted values: `'simple'`, `'full'`.
175+
* `skipPrototype` {boolean} If set to `true`, the error message does not report a
176+
mismatch of the top-level prototypes when the operator is `'deepStrictEqual'`.
177+
Defaults to `false`.
175178

176179
A subclass of {Error} that indicates the failure of an assertion.
177180

‎lib/assert.js‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -315,6 +315,9 @@ Assert.prototype.deepStrictEqual = function deepStrictEqual(actual, expected, ..
315315
operator: 'deepStrictEqual',
316316
stackStartFn: deepStrictEqual,
317317
diff: this?.[kOptions]?.diff,
318+
// The message must not report a prototype mismatch as the cause of the
319+
// failure when the comparison itself ignored the prototypes.
320+
skipPrototype: this?.[kOptions]?.skipPrototype,
318321
});
319322
}
320323
};

‎lib/internal/assert/assertion_error.js‎

Lines changed: 62 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
'use strict';
22

33
const {
4+
ArrayPrototype,
45
ArrayPrototypeJoin,
56
ArrayPrototypePop,
67
ArrayPrototypeSlice,
@@ -9,6 +10,7 @@ const {
910
ObjectAssign,
1011
ObjectDefineProperty,
1112
ObjectGetPrototypeOf,
13+
ObjectPrototype,
1214
ObjectPrototypeHasOwnProperty,
1315
SafeSet,
1416
String,
@@ -182,9 +184,64 @@ function isSimpleDiff(actual, inspectedActual, expected, inspectedExpected) {
182184
return typeof actual !== 'object' || actual === null || typeof expected !== 'object' || expected === null;
183185
}
184186

185-
function createErrDiff(actual, expected, operator, customMessage, diffType = 'simple') {
187+
function isNonDefaultPrototype(proto) {
188+
return proto !== null && proto !== ObjectPrototype && proto !== ArrayPrototype;
189+
}
190+
191+
// Returns a short human-readable identifier of the prototype, based on its
192+
// constructor, that is safe to derive on exotic objects.
193+
function getPrototypeName(proto) {
194+
if (proto === null) {
195+
return '(null prototype)';
196+
}
197+
try {
198+
const ctor = proto.constructor;
199+
if (typeof ctor === 'function') {
200+
// Read `name` only once and validate the value before using it: a
201+
// stateful getter may return a string on one access and a different
202+
// value (or throw) on the next one.
203+
const name = ctor.name;
204+
if (typeof name === 'string' && name !== '') {
205+
return name;
206+
}
207+
}
208+
} catch {
209+
// Ignore exotic prototypes that throw on property access.
210+
}
211+
return '(anonymous)';
212+
}
213+
214+
function createErrDiff(actual, expected, operator, customMessage, diffType = 'simple', skipPrototype = false) {
186215
operator = checkOperator(actual, expected, operator);
187216

217+
// `deepStrictEqual` requires both values to share the same prototype, but
218+
// the structural diff may not make that difference obvious (e.g. when both
219+
// values are inspected identically). Surface a mismatch of the top-level
220+
// values explicitly when at least one of the prototypes is not a default
221+
// prototype. Prototype differences of nested objects are not reported.
222+
// Refs: https://github.com/nodejs/node/issues/50397
223+
// Deriving the hint may run arbitrary code (e.g. a `getPrototypeOf` trap of
224+
// a Proxy or a throwing getter) that can throw. Such a failure must not
225+
// replace the assertion error nor leak, so the hint is simply omitted.
226+
let prototypeMismatchMessage = '';
227+
if (!skipPrototype && operator === 'deepStrictEqual' &&
228+
actual !== null && typeof actual === 'object' &&
229+
expected !== null && typeof expected === 'object') {
230+
try {
231+
const actualProto = ObjectGetPrototypeOf(actual);
232+
const expectedProto = ObjectGetPrototypeOf(expected);
233+
if (actualProto !== expectedProto &&
234+
(isNonDefaultPrototype(actualProto) ||
235+
isNonDefaultPrototype(expectedProto))) {
236+
prototypeMismatchMessage =
237+
`\nObject prototypes differ: ${getPrototypeName(actualProto)}` +
238+
` !== ${getPrototypeName(expectedProto)}`;
239+
}
240+
} catch {
241+
// Omit the hint when it cannot be derived safely.
242+
}
243+
}
244+
188245
let skipped = false;
189246
let message = '';
190247
const inspectedActual = inspectValue(actual);
@@ -232,7 +289,7 @@ function createErrDiff(actual, expected, operator, customMessage, diffType = 'si
232289
const headerMessage = `${getErrorMessage(operator, customMessage)}\n${header}`;
233290
const skippedMessage = skipped ? '\n... Skipped lines' : '';
234291

235-
return `${headerMessage}${skippedMessage}\n${message}\n`;
292+
return `${headerMessage}${skippedMessage}\n${message}${prototypeMismatchMessage}\n`;
236293
}
237294

238295
function addEllipsis(string) {
@@ -257,6 +314,7 @@ class AssertionError extends Error {
257314
// Compatibility with older versions.
258315
stackStartFunction,
259316
diff = 'simple',
317+
skipPrototype = false,
260318
} = options;
261319
let {
262320
actual,
@@ -268,7 +326,7 @@ class AssertionError extends Error {
268326

269327
if (message != null) {
270328
if (kMethodsWithCustomMessageDiff.has(operator)) {
271-
super(createErrDiff(actual, expected, operator, message, diff));
329+
super(createErrDiff(actual, expected, operator, message, diff, skipPrototype));
272330
} else {
273331
super(String(message));
274332
}
@@ -288,7 +346,7 @@ class AssertionError extends Error {
288346
}
289347

290348
if (kMethodsWithCustomMessageDiff.has(operator)) {
291-
super(createErrDiff(actual, expected, operator, message, diff));
349+
super(createErrDiff(actual, expected, operator, message, diff, skipPrototype));
292350
} else if (operator === 'notDeepStrictEqual' ||
293351
operator === 'notStrictEqual') {
294352
// In case the objects are equal but the operator requires unequal, show

‎lib/internal/assert/utils.js‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,9 @@ const escapeFn = (str) => meta[StringPrototypeCharCodeAt(str, 0)];
7373
* @property {string} operator Operator
7474
* @property {Function} stackStartFn Stack start function
7575
* @property {'simple' | 'full'} [diff] Diff mode
76+
* @property {boolean} [skipPrototype] Set to `true` when the comparison
77+
* ignored the prototypes, so the message does not report a prototype
78+
* mismatch as the cause of the failure
7679
* @property {boolean} [generatedMessage] Generated message
7780
*/
7881

‎test/parallel/test-assert-deep.js‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,8 @@ test('deepEqual', () => {
7878
' 121,\n' +
7979
' 122,\n' +
8080
' 10\n' +
81-
' ]\n'
81+
' ]\n' +
82+
'Object prototypes differ: Uint8Array !== Buffer\n'
8283
}
8384
);
8485
assert.deepEqual(arr, buf);
@@ -138,7 +139,8 @@ test('date', () => {
138139
code: 'ERR_ASSERTION',
139140
message: `${defaultMsgStartFull}\n\n` +
140141
'+ 2016-01-01T00:00:00.000Z\n- MyDate 2016-01-01T00:00:00.000Z' +
141-
" {\n- '0': '1'\n- }\n"
142+
" {\n- '0': '1'\n- }\n" +
143+
'Object prototypes differ: Date !== MyDate\n'
142144
}
143145
);
144146
assert.throws(
@@ -147,7 +149,8 @@ test('date', () => {
147149
code: 'ERR_ASSERTION',
148150
message: `${defaultMsgStartFull}\n\n` +
149151
'+ MyDate 2016-01-01T00:00:00.000Z {\n' +
150-
"+ '0': '1'\n+ }\n- 2016-01-01T00:00:00.000Z\n"
152+
"+ '0': '1'\n+ }\n- 2016-01-01T00:00:00.000Z\n" +
153+
'Object prototypes differ: MyDate !== Date\n'
151154
}
152155
);
153156
});
@@ -162,7 +165,8 @@ test('regexp', () => {
162165
{
163166
code: 'ERR_ASSERTION',
164167
message: `${defaultMsgStartFull}\n\n` +
165-
"+ /test/\n- MyRegExp /test/ {\n- '0': '1'\n- }\n"
168+
"+ /test/\n- MyRegExp /test/ {\n- '0': '1'\n- }\n" +
169+
'Object prototypes differ: RegExp !== MyRegExp\n'
166170
}
167171
);
168172
});

0 commit comments

Comments
 (0)