Skip to content

Fix/no in place expansion of caller schema - #291

Merged
fredbi merged 3 commits into
go-openapi:masterfrom
fredbi:fix/no-in-place-expansion-of-caller-schema
Aug 24, 2026
Merged

Fix/no in place expansion of caller schema#291
fredbi merged 3 commits into
go-openapi:masterfrom
fredbi:fix/no-in-place-expansion-of-caller-schema

Conversation

@fredbi

@fredbi fredbi commented Aug 24, 2026

Copy link
Copy Markdown
Member

Change type

Please select: 🆕 New feature or enhancement|🔧 Bug fix'|📃 Documentation update

Short description

Fixes

Full description

Checklist

  • I have signed all my commits with my name and email (see DCO. This does not require a PGP-signed commit
  • I have rebased and squashed my work, so only one commit remains
  • I have added tests to cover my changes.
  • I have properly enriched go doc comments in code.
  • I have properly documented any breaking change.

fredbi and others added 3 commits August 24, 2026 19:51
deepCloneSchema round-tripped through gob, which omits any field holding
its zero value and flattens a pointer to what it points at. A *float64
pointing at 0 therefore travelled as the zero value and came back nil, so
"minimum": 0 was dropped - and the JSON Schema meta-schema spells every
positiveInteger that way, which is what maxLength, maxItems, minLength and
multipleOf resolve to. The one caller clones the meta-schema's
#/definitions/parameter before expanding it, so parameter validation has
been checking against a meta-schema with those lower bounds missing.

The copy now goes through jsonutils.FromDynamicJSON, the form spec.Schema
is defined by. On a mid-sized document JSON also round-trips faster than
gob and allocates less than half as much, so nothing is traded for the
correctness.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Frederic BIDON <fredbi@yahoo.com>
newSchemaValidator expands a schema that carries a $ref or an id, and
spec.ExpandSchemaWithOptions rewrites the schema it is given. Several
callers hand it a schema that belongs to the document the caller still
holds: the members of allOf/anyOf/oneOf, "not", a dependency schema,
additionalProperties, and every schema the default and example validators
walk out of the specification. Validating replaced those $ref with their
targets, so a caller that went on to flatten the document worked on a
different document than the one it loaded - go-swagger reloads the spec
after validating for exactly this reason.

SpecValidator.Validate now takes one copy of the document and works on
that. Expansion below is unchanged, so what validation reports is
unchanged too, down to the error paths. Raw() is read before the copy, so
the checks that go through the authored bytes still see them.

NewSchemaValidator does the same for the single-schema API. Both mark
SchemaValidatorOptions.ownSchemata, which tells the validators beneath
that the schemata they walk are theirs to rewrite.

The copy is taken at the entry point rather than in newSchemaValidator
because a copy per schema is paid again at every level of a recursive
document: on the kubernetes benchmark that cost 4x the time and 3.3x the
memory. Cloning once measures as no change (benchstat p=0.69, n=5). It
also has to cover parameters, path items and responses, which expand as
well.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Frederic BIDON <fredbi@yahoo.com>
Signed-off-by: Frederic BIDON <fredbi@yahoo.com>
@codecov

codecov Bot commented Aug 24, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 84.61538% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 96.92%. Comparing base (bec3b87) to head (bbce87e).
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
schema.go 75.00% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #291      +/-   ##
==========================================
- Coverage   96.94%   96.92%   -0.03%     
==========================================
  Files          34       34              
  Lines        3763     3771       +8     
==========================================
+ Hits         3648     3655       +7     
- Misses        110      111       +1     
  Partials        5        5              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

@fredbi
fredbi merged commit d6d7bac into go-openapi:master Aug 24, 2026
24 checks passed
@fredbi
fredbi deleted the fix/no-in-place-expansion-of-caller-schema branch August 24, 2026 19:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant