diff --git a/lib/types/keys.js b/lib/types/keys.js index a920afa4..233a470c 100755 --- a/lib/types/keys.js +++ b/lib/types/keys.js @@ -602,7 +602,23 @@ module.exports = Any.extend({ if (schema.$_terms.keys) { const topo = new Topo.Sorter(); for (const child of schema.$_terms.keys) { - Common.tryWithPath(() => topo.add(child, { after: child.schema.$_rootReferences(), group: child.key }), child.key); + Common.tryWithPath(() => topo.add(child, { after: child.schema.$_rootReferences(), group: child.key, manual: true }), child.key); + } + + try { + topo.sort(); + } + catch (err) { + // Sorting failed (e.g. circular references). Replay the adds one at a + // time, sorting after each, to identify the offending key in the error. + const detailed = new Topo.Sorter(); + for (const child of schema.$_terms.keys) { + Common.tryWithPath(() => detailed.add(child, { after: child.schema.$_rootReferences(), group: child.key }), child.key); + } + + // The replay above always throws first - this is an unreachable safety net + + throw err; // $lab:coverage:ignore$ } schema.$_terms.keys = new internals.Keys(...topo.nodes); diff --git a/test/types/object.js b/test/types/object.js index 88ae3bd6..dcc9207f 100755 --- a/test/types/object.js +++ b/test/types/object.js @@ -1638,6 +1638,31 @@ describe('object', () => { [{ type: 'a', set: true, flag: true }, false, '"flag" must be [false]'] ]); }); + + it('errors on circular references between sibling keys', () => { + + const err = expect(() => { + + Joi.object({ + a: Joi.ref('b'), + b: Joi.ref('a') + }); + }).to.throw('item added into group b created a dependencies error'); + + expect(err.path).to.equal('b'); + }); + + it('errors on circular references introduced by rebuild', () => { + + const schema = Joi.object({ + a: Joi.any(), + b: Joi.any() + }) + .fork('a', (s) => s.default(Joi.ref('b'))); + + const err = expect(() => schema.fork('b', (s) => s.default(Joi.ref('a')))).to.throw('item added into group a created a dependencies error'); + expect(err.path).to.equal('a'); + }); }); describe('length()', () => {