oscerd commented on PR #28183: URL: https://github.com/apache/camel/pull/28183#issuecomment-6478888014
Thanks Claus. All three points are fixed in "CAMEL-25887: camel-rest-openapi - end the reference check on a cycle with a relative $id, and check a fragment against the node it selects". The two copies of `OpenApiExternalReferences` are still identical apart from the package and the class comment. **1. A cycle with a relative `$id`.** I reproduced it first. With your `node.yaml`, and with a two-document cycle that has `$id: sub/` on one side, the check recursed until a `StackOverflowError` in both modules. It now ends a path where the parser ends it. `OpenAPI31Traverser.traverseSchema` compares the schemas it visits by value (`Schema.equals`/`hashCode`), so it stops at a `$ref` equal to one it is already resolving. The check keeps the objects with the `$ref` that lead to the part it is checking, and does not follow a reference equal to one of them. I did not stop at the same document and fragment on the chain, because the parser does not always stop there. When the schema of the first visit has changed before the cycle (a sibling `$ref` resolved first), the second deserialization no longer equals it. The parser then walks the document a second time, below the `$id` it added, and loads from there before it stops at the repeated `$ref` inside. `allResolvesTheSecondVisitOfACycleBelowARelativeId` shows the parser fetching from that second walk. `localChecksTheSecondVisitOfACycleBelowARelativeId` rejects it with zero requests; with a stop at the repeated document it lets the contract through. A path holds each distinct reference at most once, so the check ends even when the bases never repeat, and no limit on the length of the chain is needed. Tests: `allLoadsASchemaThatReferencesItselfBelowARelativeId`, `localFinishesForASchemaThatReferencesItselfBelowARelativeId`, `allLoadsACycleOfTwoDocumentsWithARelativeId`, `localFinishesForACycleOfTwoDocumentsWithARelativeId` and the two second-walk tests. camel-openapi-java has the self-cycle under the default and `local`, the two-document cycle and the second walk. **2. Fragments.** For OpenAPI 3.1 the check now selects the node the fragment selects, with `ReferenceUtils.jsonPointerEvaluate` and then the `$anchor` search of `findAnchor`. It uses the tree `ReferenceVisitor.deserializeIntoTree` builds for a document it reads, and scans only that node, against the inherited `$id`s folded onto the document URI. The visited key includes the fragment, and a fragment that cannot be found fails the check. Scanning one node means the check no longer sees the rest of the document, which the whole-document scan used to cover. Two things follow from that: - The `#…` references inside a referenced document. The parser resolves them against the `$id`s in effect and looks them up in that document, so the check now follows them like the other references. This also catches a case the whole-document scan missed: `$defs.X` with an http `$id` that references `#/$defs/Y`, where the parser resolves the relative `owner.yaml` of Y against the `$id` of X. - The parser keeps a document under its location and under the base of each `$id` it meets, and takes whichever it met first. A location that an `$id` in another document also resolves to can therefore be answered from that other document, whose other nodes the check no longer reads. The check records both, and rejects a reference when another document could answer its lookup. Tests: your (a) is `localChecksAFragmentAgainstTheInheritedIdOnly` (zero requests; the `all` control fetches `/schemas/owner.yaml`). Your (b) is `localChecksAFragmentWithoutTheIdsAboveIt`, with `localChecksTheSchemaAnAnchorSelects` for the anchor: the check rejected both before, and both now load with zero requests. Then `localFailsWhenAFragmentCannotBeFound`, and `localFollowsAReferenceInsideAReferencedDocument` and `localRejectsAReferenceToADocumentThatAnIdElsewhereClaims`, each with an `all` control that fetches. The `local` cases are in camel-openapi-java too. **3. The key.** The visited key now uses the inherited `$id`s from the last absolute one on. In your example (`[]` against `[file:///d/s.yaml]`), when that `$id` sits in another document above the reference to `s.yaml`, the lookup rule from point 2 now rejects the reference, because the `$id` resolves to the location of `s.yaml`. The test therefore uses a relative `$id` instead: `a/entry.yaml` reaches `s.yaml` twice, once below `$id: ../<dir>/s.yaml`, which gives the base of `s.yaml` at `s.yaml` itself but not at `other/t.yaml`, which `s.yaml` references below `$id: sub/`. That is `localChecksADocumentReachedAgainWithIdsThatGiveTheSameBaseThere`, in both modules. Its `all` control fetches through the second visit, because the parser walks `properties` before `allOf`. Docs: the component page and the contract-first page now mention the fragment and the new failures. Verification: the reference-policy tests are 46 in camel-rest-openapi (17 new) and 20 in camel-openapi-java (11 new). Each new `local` test fails when the change it covers is disabled: no stop at a repeated reference (the check does not end), a stop at the repeated document instead (the second walk passes), the whole document scanned again, no following of `#…` references, no lookup rule, and the old key. The full suites pass (camel-rest-openapi 232 tests, camel-openapi-java 119), and so does the full reactor build (`install -DskipTests -DskipITs`). The only file it regenerated is the catalog copy of the component page, which is in the commit. _Claude Code on behalf of oscerd_ -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
