Skip to content

Commit d683aeb

Browse files
mdjermanovicmdjermanovic
authored
fix: don't crash on tests with circular references in RuleTester (#19664)
Fixes #19646
1 parent f1c858e commit d683aeb

4 files changed

Lines changed: 238 additions & 9 deletions

File tree

‎lib/rule-tester/rule-tester.js‎

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -192,16 +192,24 @@ function cloneDeeplyExcludesParent(x) {
192192
/**
193193
* Freezes a given value deeply.
194194
* @param {any} x A value to freeze.
195+
* @param {Set<Object>} seenObjects Objects already seen during the traversal.
195196
* @returns {void}
196197
*/
197-
function freezeDeeply(x) {
198+
function freezeDeeply(x, seenObjects = new Set()) {
198199
if (typeof x === "object" && x !== null) {
200+
if (seenObjects.has(x)) {
201+
return; // skip to avoid infinite recursion
202+
}
203+
seenObjects.add(x);
204+
199205
if (Array.isArray(x)) {
200-
x.forEach(freezeDeeply);
206+
x.forEach(element => {
207+
freezeDeeply(element, seenObjects);
208+
});
201209
} else {
202210
for (const key in x) {
203211
if (key !== "parent" && hasOwnProperty(x, key)) {
204-
freezeDeeply(x[key]);
212+
freezeDeeply(x[key], seenObjects);
205213
}
206214
}
207215
}

‎lib/shared/serialization.js‎

Lines changed: 29 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -26,21 +26,44 @@ function isSerializablePrimitiveOrPlainObject(val) {
2626
* Check if a value is serializable.
2727
* Functions or objects like RegExp cannot be serialized by JSON.stringify().
2828
* Inspired by: https://stackoverflow.com/questions/30579940/reliable-way-to-check-if-objects-is-serializable-in-javascript
29-
* @param {any} val the value
30-
* @returns {boolean} true if the value is serializable
29+
* @param {any} val The value
30+
* @param {Set<Object>} seenObjects Objects already seen in this path from the root object.
31+
* @returns {boolean} `true` if the value is serializable
3132
*/
32-
function isSerializable(val) {
33+
function isSerializable(val, seenObjects = new Set()) {
3334
if (!isSerializablePrimitiveOrPlainObject(val)) {
3435
return false;
3536
}
36-
if (typeof val === "object") {
37+
if (typeof val === "object" && val !== null) {
38+
if (seenObjects.has(val)) {
39+
/*
40+
* Since this is a depth-first traversal, encountering
41+
* the same object again means there is a circular reference.
42+
* Objects with circular references are not serializable.
43+
*/
44+
return false;
45+
}
3746
for (const property in val) {
3847
if (Object.hasOwn(val, property)) {
3948
if (!isSerializablePrimitiveOrPlainObject(val[property])) {
4049
return false;
4150
}
42-
if (typeof val[property] === "object") {
43-
if (!isSerializable(val[property])) {
51+
if (
52+
typeof val[property] === "object" &&
53+
val[property] !== null
54+
) {
55+
if (
56+
/*
57+
* We're creating a new Set of seen objects because we want to
58+
* ensure that `val` doesn't appear again in this path, but it can appear
59+
* in other paths. This allows for resuing objects in the graph, as long as
60+
* there are no cycles.
61+
*/
62+
!isSerializable(
63+
val[property],
64+
new Set([...seenObjects, val]),
65+
)
66+
) {
4467
return false;
4568
}
4669
}

‎tests/lib/rule-tester/rule-tester.js‎

Lines changed: 172 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4565,6 +4565,79 @@ describe("RuleTester", () => {
45654565
}, "detected duplicate test case");
45664566
});
45674567

4568+
it("throws with duplicate object test cases when they are the same object", () => {
4569+
const test = { code: "foo" };
4570+
assert.throws(() => {
4571+
ruleTester.run(
4572+
"foo",
4573+
{
4574+
meta: {},
4575+
create() {
4576+
return {};
4577+
},
4578+
},
4579+
{
4580+
valid: [test, test],
4581+
invalid: [],
4582+
},
4583+
);
4584+
}, "detected duplicate test case");
4585+
});
4586+
4587+
it("throws with duplicate object test cases that have multiple references to the same object", () => {
4588+
const obj1 = { foo: { bar: "baz" } };
4589+
const obj2 = { foo: { bar: "baz" } };
4590+
4591+
assert.throws(() => {
4592+
ruleTester.run(
4593+
"foo",
4594+
{
4595+
meta: {},
4596+
create() {
4597+
return {};
4598+
},
4599+
},
4600+
{
4601+
valid: [
4602+
{
4603+
code: "foo",
4604+
settings: { qux: obj1, quux: obj1 },
4605+
},
4606+
{
4607+
code: "foo",
4608+
settings: { qux: obj2, quux: obj2 },
4609+
},
4610+
],
4611+
invalid: [],
4612+
},
4613+
);
4614+
}, "detected duplicate test case");
4615+
});
4616+
4617+
it("does not throw with duplicate object test cases that have circular references", () => {
4618+
const obj1 = { foo: "bar" };
4619+
obj1.circular = obj1;
4620+
const obj2 = { foo: "bar" };
4621+
obj2.circular = obj2;
4622+
4623+
ruleTester.run(
4624+
"foo",
4625+
{
4626+
meta: {},
4627+
create() {
4628+
return {};
4629+
},
4630+
},
4631+
{
4632+
valid: [
4633+
{ code: "foo", settings: { baz: obj1 } },
4634+
{ code: "foo", settings: { baz: obj2 } },
4635+
],
4636+
invalid: [],
4637+
},
4638+
);
4639+
});
4640+
45684641
it("throws with string and object test cases", () => {
45694642
assert.throws(() => {
45704643
ruleTester.run(
@@ -4683,6 +4756,105 @@ describe("RuleTester", () => {
46834756
}, "detected duplicate test case");
46844757
});
46854758

4759+
it("throws with duplicate object test cases when they are the same object", () => {
4760+
const test = {
4761+
code: "const x = 123;",
4762+
errors: [{ message: "foo bar" }],
4763+
};
4764+
4765+
assert.throws(() => {
4766+
ruleTester.run(
4767+
"foo",
4768+
{
4769+
meta: {},
4770+
create(context) {
4771+
return {
4772+
VariableDeclaration(node) {
4773+
context.report(node, "foo bar");
4774+
},
4775+
};
4776+
},
4777+
},
4778+
{
4779+
valid: ["foo"],
4780+
invalid: [test, test],
4781+
},
4782+
);
4783+
}, "detected duplicate test case");
4784+
});
4785+
4786+
it("throws with duplicate object test cases that have multiple references to the same object", () => {
4787+
const obj1 = { foo: { bar: "baz" } };
4788+
const obj2 = { foo: { bar: "baz" } };
4789+
4790+
assert.throws(() => {
4791+
ruleTester.run(
4792+
"foo",
4793+
{
4794+
meta: {},
4795+
create(context) {
4796+
return {
4797+
VariableDeclaration(node) {
4798+
context.report(node, "foo bar");
4799+
},
4800+
};
4801+
},
4802+
},
4803+
{
4804+
valid: ["foo"],
4805+
invalid: [
4806+
{
4807+
code: "const x = 123;",
4808+
settings: { qux: obj1, quux: obj1 },
4809+
errors: [{ message: "foo bar" }],
4810+
},
4811+
{
4812+
code: "const x = 123;",
4813+
settings: { qux: obj2, quux: obj2 },
4814+
errors: [{ message: "foo bar" }],
4815+
},
4816+
],
4817+
},
4818+
);
4819+
}, "detected duplicate test case");
4820+
});
4821+
4822+
it("does not throw with duplicate object test cases that have circular references", () => {
4823+
const obj1 = { foo: "bar" };
4824+
obj1.circular = obj1;
4825+
const obj2 = { foo: "bar" };
4826+
obj2.circular = obj2;
4827+
4828+
ruleTester.run(
4829+
"foo",
4830+
{
4831+
meta: {},
4832+
create(context) {
4833+
return {
4834+
VariableDeclaration(node) {
4835+
context.report(node, "foo bar");
4836+
},
4837+
};
4838+
},
4839+
},
4840+
{
4841+
valid: ["foo"],
4842+
invalid: [
4843+
{
4844+
code: "const x = 123;",
4845+
settings: { baz: obj1 },
4846+
errors: [{ message: "foo bar" }],
4847+
},
4848+
{
4849+
code: "const x = 123;",
4850+
settings: { baz: obj2 },
4851+
errors: [{ message: "foo bar" }],
4852+
},
4853+
],
4854+
},
4855+
);
4856+
});
4857+
46864858
it("throws with duplicate object test cases when options is a primitive", () => {
46874859
assert.throws(() => {
46884860
ruleTester.run(

‎tests/lib/shared/serialization.js‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,32 @@ describe("serialization", () => {
6868
assert.isFalse(isSerializable({ a: /abc/u }));
6969
assert.isFalse(isSerializable({ a: { b: /abc/u } }));
7070
});
71+
72+
it("circular references", () => {
73+
const obj1 = {};
74+
obj1.circular = obj1;
75+
assert.isFalse(isSerializable(obj1));
76+
assert.isFalse(isSerializable({ a: obj1 }));
77+
78+
const obj2 = {};
79+
obj2.a = { circular: obj2 };
80+
assert.isFalse(isSerializable(obj2));
81+
assert.isFalse(isSerializable({ b: obj2 }));
82+
83+
const obj3 = { foo: { bar: "baz" } };
84+
assert.isTrue(
85+
isSerializable({
86+
a: obj3,
87+
b: obj3,
88+
c: {
89+
d: obj3,
90+
e: {
91+
f: obj3,
92+
},
93+
},
94+
}),
95+
);
96+
});
7197
});
7298

7399
describe("array", () => {

0 commit comments

Comments
 (0)