diff --git a/lib/dereference.ts b/lib/dereference.ts index 725f703b..40faf7fd 100644 --- a/lib/dereference.ts +++ b/lib/dereference.ts @@ -220,26 +220,46 @@ function dereference$Ref 1) { + const extraKeys = {}; + for (const key of refKeys) { + if (key !== "$ref" && !(key in cache.value)) { + // @ts-expect-error TS(7053): Element implicitly has an 'any' type because expre... Remove this comment to see the full error message + extraKeys[key] = $ref[key]; + } + } + return { + circular: cache.circular, + value: Object.assign({}, cache.value, extraKeys), + }; + } + return cache; } - const refKeys = Object.keys($ref); - if (refKeys.length > 1) { - const extraKeys = {}; - for (const key of refKeys) { - if (key !== "$ref" && !(key in cache.value)) { - // @ts-expect-error TS(7053): Element implicitly has an 'any' type because expre... Remove this comment to see the full error message - extraKeys[key] = $ref[key]; - } + // If both our cached value and our incoming `$ref` are the same then we can return what we + // got out of the cache, otherwise we should re-process this value. We need to do this because + // the current dereference caching mechanism doesn't take into account that `$ref` are neither + // unique or reference the same file. + // + // For example if `schema.yaml` references `definitions/child.yaml` and + // `definitions/parent.yaml` references `child.yaml` then `$ref: 'child.yaml'` may get cached + // for `definitions/child.yaml`, resulting in `schema.yaml` being having an invalid reference + // to `child.yaml`. + // + // This check is not perfect and the design of the dereference caching mechanism needs a total + // overhaul. + if (typeof cache.value === 'object' && '$ref' in cache.value && '$ref' in $ref) { + if (cache.value.$ref === $ref.$ref) { + return cache; + } else { + // no-op } - return { - circular: cache.circular, - value: Object.assign({}, cache.value, extraKeys), - }; + } else { + return cache; } - - return cache; } const pointer = $refs._resolve($refPath, path, options); diff --git a/test/specs/circular-external/circular-external.spec.ts b/test/specs/circular-external/circular-external.spec.ts index 4cdf62f6..169fb02b 100644 --- a/test/specs/circular-external/circular-external.spec.ts +++ b/test/specs/circular-external/circular-external.spec.ts @@ -53,6 +53,28 @@ describe("Schema with circular (recursive) external $refs", () => { expect(schema.definitions.child.properties.parents.items).to.equal(schema.definitions.parent); }); + it('should throw an error if "options.dereference.circular" is "ignore"', async () => { + const parser = new $RefParser(); + + const schema = await parser.dereference(path.rel("test/specs/circular-external/circular-external.yaml"), { + dereference: { circular: 'ignore' }, + }); + + expect(schema).to.equal(parser.schema); + expect(schema).not.to.deep.equal(dereferencedSchema); + expect(parser.$refs.circular).to.equal(true); + // @ts-expect-error TS(2532): Object is possibly 'undefined'. + expect(schema.definitions.pet.title).to.equal('pet'); + // @ts-expect-error TS(2532): Object is possibly 'undefined'. + expect(schema.definitions.thing).to.deep.equal({ $ref: 'circular-external.yaml#/definitions/thing'}); + // @ts-expect-error TS(2532): Object is possibly 'undefined'. + expect(schema.definitions.person).to.deep.equal({ $ref: 'definitions/person.yaml'}); + // @ts-expect-error TS(2532): Object is possibly 'undefined'. + expect(schema.definitions.parent).to.deep.equal({ $ref: 'definitions/parent.yaml'}); + // @ts-expect-error TS(2532): Object is possibly 'undefined'. + expect(schema.definitions.child).to.deep.equal({ $ref: 'definitions/child.yaml'}); + }); + it('should throw an error if "options.dereference.circular" is false', async () => { const parser = new $RefParser();