Skip to content

fix(dereference): cache poisoning when dereferencing external schemas #381

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 2 commits into from
Apr 12, 2025

Conversation

erunion
Copy link
Contributor

@erunion erunion commented Apr 12, 2025

Apologies for the fast follow here but I uncovered a bug in #380 while incorporating v12 into our OpenAPI parser. When dereferencing schemas have external references (eg. $ref: definitions/child.yaml) that $ref could get saved into the dereference cache as child.yaml resulting in the parent schema ending up with an invalid $ref pointer that would be child.yaml:

screenshot_2025-04-11_at_5 12 33___pm

This case unfortunately was not caught because the circular-external test suite did not have a case for dereference.circular=ignore.

I loathe the fix I am submitting here but it resolves this, very unique, case and as we both agree in #380 that the current dereference caching system needs a completely overhaul.

@erunion erunion changed the title Fix/external cache poisoning fix(dereference): cache poisoning when dereferencing external schemas Apr 12, 2025
Copy link
Contributor Author

@erunion erunion Apr 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might want to review this with whitespace changes off, I moved some things around because this cache logic was beginning to make my head spin. 🫨

@coveralls
Copy link

Pull Request Test Coverage Report for Build 14414415087

Details

  • 18 of 20 (90.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.07%) to 92.919%

Changes Missing Coverage Covered Lines Changed/Added Lines %
lib/dereference.ts 18 20 90.0%
Totals Coverage Status
Change from base Build 14413869142: 0.07%
Covered Lines: 1651
Relevant Lines: 1751

💛 - Coveralls

@jonluca jonluca merged commit 69cfcf6 into APIDevTools:main Apr 12, 2025
14 checks passed
Copy link

🎉 This PR is included in version 12.0.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants