feat: OpenAPI 3.1 support with scoped validation enhancements#1
Open
AlinsRan wants to merge 23 commits intoapi7:masterfrom
Open
feat: OpenAPI 3.1 support with scoped validation enhancements#1AlinsRan wants to merge 23 commits intoapi7:masterfrom
AlinsRan wants to merge 23 commits intoapi7:masterfrom
Conversation
Add comprehensive OpenAPI 3.1 / JSON Schema 2020-12 support: - Type arrays with null support (e.g., ["string", "null"]) - JSON Schema 2020-12 keywords: const, examples, prefixItems, contains, minContains, maxContains, patternProperties, dependentSchemas, propertyNames, unevaluatedItems, unevaluatedProperties - Conditional keywords: if/then/else, dependentRequired - ExclusiveBound union type for 3.0 (boolean) and 3.1 (numeric) exclusive bounds - Webhooks support with $ref resolution - Version detection helpers: IsOpenAPI3_0(), IsOpenAPI3_1() - Info.Summary and License.Identifier fields - JSON Schema 2020-12 validator via EnableJSONSchema2020() - Document-level validation option: EnableJSONSchema2020Validation() - Allow $ref alongside other keywords in 3.1 schemas - Handle "null" type in schema validation - Auto-detect 3.1 in cmd/validate Co-Authored-By: Chance Kirsch <> Co-Authored-By: RobbertDM <> Co-Authored-By: Claude Opus 4.6 <[email protected]>
…, etc.) The loader's resolveSchemaRef only resolved $ref in pre-3.1 fields (items, properties, additionalProperties, not, allOf, anyOf, oneOf). References inside the new OpenAPI 3.1 / JSON Schema 2020-12 fields were silently left unresolved, causing nil Value pointers. This adds ref resolution for: prefixItems, contains, patternProperties, dependentSchemas, propertyNames, unevaluatedItems, unevaluatedProperties. Co-Authored-By: Claude Opus 4.6 <[email protected]>
The OpenAPI-to-JSON-Schema transformation only recursed into pre-3.1 fields (properties, additionalProperties, items, not, oneOf, anyOf, allOf). Nested schemas inside 3.1 fields with OpenAPI 3.0-isms like nullable:true were not converted, causing incorrect validation. This adds recursion into: prefixItems, contains, patternProperties, dependentSchemas, propertyNames, unevaluatedItems, unevaluatedProperties. Also consolidates the properties/patternProperties/dependentSchemas map iteration into a single loop. Co-Authored-By: Claude Opus 4.6 <[email protected]>
Schema.Validate() (used by doc.Validate()) recursively validates sub-schemas in Items, Properties, AdditionalProperties, etc. but did not recurse into the new OpenAPI 3.1 / JSON Schema 2020-12 fields. Invalid sub-schemas nested inside these fields went undetected. This adds validation for: prefixItems, contains, patternProperties, dependentSchemas, propertyNames, unevaluatedItems, unevaluatedProperties. Co-Authored-By: Claude Opus 4.6 <[email protected]>
Co-Authored-By: Claude Opus 4.6 <[email protected]>
- Make paths optional in 3.1 (required only in 3.0) - Add mutualTLS security scheme type validation - Validate license url/identifier mutual exclusivity - Enable JSON Schema 2020-12 validation in openapi3filter for 3.1 docs Co-Authored-By: Claude Opus 4.6 <[email protected]>
- Add $id, $anchor, $dynamicRef, $dynamicAnchor identity keywords - Add contentMediaType, contentEncoding, contentSchema vocabulary - Add discriminator support for anyOf (was only oneOf) - Validate jsonSchemaDialect as valid URI Co-Authored-By: Claude Opus 4.6 <[email protected]>
Co-Authored-By: Claude Opus 4.6 <[email protected]>
- IsEmpty(): add missing checks for PrefixItems, Contains, MinContains, MaxContains, PatternProperties, DependentSchemas, PropertyNames, UnevaluatedItems, UnevaluatedProperties, Examples - JSONLookup(): add all 23 missing JSON Schema 2020-12 field cases - validate(): relax items requirement for arrays when in 3.1 mode or when prefixItems is present - transformOpenAPIToJSONSchema: clean up exclusiveMinimum/Maximum false, handle nullable:true without type field - MarshalYAML: only emit paths when non-nil (valid in 3.1) - visitConstOperation: use reflect.DeepEqual for json.Number comparison - Webhooks validation: use componentNames() for deterministic ordering Co-Authored-By: Claude Opus 4.6 <[email protected]>
Rephrase text to not contain literal json struct tag syntax that triggers the json/yaml tag consistency check. Co-Authored-By: Claude Opus 4.6 <[email protected]>
Open issues are tracked in the PR getkin#1125 description instead. Co-Authored-By: Claude Opus 4.6 <[email protected]>
- Add $comment keyword to Schema struct (MarshalYAML, UnmarshalJSON, IsEmpty, JSONLookup) - Fix PrefixItems type from []*SchemaRef to SchemaRefs for consistency with OneOf/AnyOf/AllOf and JSON Pointer support - Fix exclusiveBoundToBool data loss: preserve numeric bound value when converting OAS 3.1 exclusive bounds to OAS 2.0 - Auto-enable JSON Schema 2020-12 validation for OpenAPI 3.1 documents in doc.Validate() so library users don't need explicit opt-in - Add ref resolution tests for if/then/else and contentSchema - Add transform test for contentSchema with nullable nested schema - Add validate test for contentSchema with invalid sub-schema - Document breaking API changes in README (ExclusiveBound, PrefixItems) - Regenerate docs Co-Authored-By: Claude Opus 4.6 <[email protected]>
- Add $schema keyword to Schema struct for per-schema dialect declaration - Add $defs keyword (Schemas map) for local reusable schema definitions, with full support: struct, marshal, unmarshal, IsEmpty, JSONLookup, validate (recurse), loader (resolve refs), transform (recurse) - Fix jsonSchemaDialect URI validation to require a scheme - Refactor discriminator resolution into shared helper to eliminate code duplication between oneOf and anyOf paths - Regenerate docs Co-Authored-By: Claude Opus 4.6 <[email protected]>
OpenAPI 3.1 adds Const (any) and Examples ([]any) fields to Schema. Like Enum/Default/Example, these can contain arbitrary JSON/YAML values that pick up __origin__ metadata from the YAML loader. Strip it on unmarshal to prevent false diffs and unexpected metadata in parsed values. Adds TestOrigin_ConstAndExamplesStripped regression test. Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
The origin-tracking YAML loader injects __origin__ as a key inside any map-valued field to record source location. However, typed Go maps (map[string]*Encoding, map[string]*ServerVariable, map[string]*PathItem) treat __origin__ as a real entry, causing false positive diffs when the same spec is loaded from two different file paths. Fix by deleting originKey from these three maps after JSON unmarshaling, mirroring the existing pattern used for Extensions and the unmarshalStringMapP helper already used by Content, Schemas, Headers, etc. Affected: - MediaType.Encoding (map[string]*Encoding) - Server.Variables (map[string]*ServerVariable) - T.Webhooks (map[string]*PathItem) Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
Implement scoped OpenAPI 3.1 features for CGo validation library: - P0-1: Fall back to built-in validator when JSON Schema 2020-12 compilation fails (e.g. unresolved cross-schema $ref) - P0-2: Merge $ref sibling keywords (description, title, default, example, nullable, etc.) for OpenAPI 3.1 documents only - P1-3: Add patternProperties validation in visitJSONObject with proper regex matching and error handling - P1-4: Add prefixItems positional validation in visitJSONArray - P1-5: Fix const:null ambiguity via ConstIsSet field detection in UnmarshalJSON, supporting both JSON-parsed and programmatic schemas - P1-6: Treat format as annotation-only in OpenAPI 3.1 mode - P2-7: Add $anchor fragment resolution in loader - P2-8: Add $dynamicAnchor indexing in loader Also fixes: - nullable transform uses []any instead of []string for jsonschema compiler compatibility - Remove unsafe pointer-based validator cache that caused stale validator reuse across GC cycles Ref: #2
|
Foo Bar seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
📝 WalkthroughWalkthroughAdded OpenAPI 3.1 / JSON Schema 2020-12 support across the library: new ExclusiveBound type, many Schema fields and helpers, a JSON‑Schema 2020-12 validator path, enhanced loader/ref resolution and anchor indexing, Components.PathItems and webhooks, version detection helpers, and conditional runtime enabling of 2020-12 validation. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client
participant Doc as openapi3.T
participant Schema as openapi3.Schema
participant JS as jsonschema.Compiler
Client->>Doc: doc.Validate(ctx, opts...)
Doc->>Doc: IsOpenAPI3_1()
alt OpenAPI 3.1 detected
Doc->>Doc: append EnableJSONSchema2020Validation()
end
Client->>Schema: Schema.VisitJSON(value, opts...)
Schema->>Schema: check useJSONSchema2020 option
alt JSON Schema 2020-12 enabled
Schema->>JS: newJSONSchemaValidator(transform(schema))
JS->>JS: compile schema (2020-12)
JS->>JS: validate(value)
JS-->>Schema: errors or nil
else built-in validator
Schema->>Schema: run built-in VisitJSON checks
Schema-->>Client: errors or nil
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Nitpick comments (13)
openapi3/openapi3_version_test.go (2)
11-11: Package-levelctxvariable shadows local test contexts.Declaring
ctxat package level can cause confusion and potential issues if tests are run in parallel or if individual tests need different context configurations. Consider declaringctxlocally within each test function that needs it.♻️ Proposed refactor
-var ctx = context.Background() - func TestDocumentVersionDetection(t *testing.T) {Then add local
ctx := context.Background()in functions that need it, such as:func TestWebhooksField(t *testing.T) { // ... t.Run("validate webhooks", func(t *testing.T) { ctx := context.Background() // ... err := doc.Validate(ctx)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@openapi3/openapi3_version_test.go` at line 11, Remove the package-level ctx variable and instead declare a local context inside each test that needs it (e.g., add ctx := context.Background() at the top of TestWebhooksField and inside subtests that call doc.Validate(ctx)); update any references to the package-level ctx to use the local ctx so tests do not share or shadow a global context.
193-217: Test webhook operation is missingResponsesfield.The webhook
Postoperation at line 203-206 doesn't have aResponsesfield. While this works for theJSONLookuptest, it would fail validation ifdoc.Validate(ctx)were called on this document. This inconsistency could mask issues if the test is later extended.🧪 Proposed fix for consistency
doc := &T{ OpenAPI: "3.1.0", Info: &Info{ Title: "Test API", Version: "1.0.0", }, Paths: NewPaths(), Webhooks: map[string]*PathItem{ "test": { Post: &Operation{ Summary: "Test webhook", + Responses: NewResponses( + WithStatus(200, &ResponseRef{ + Value: &Response{ + Description: Ptr("OK"), + }, + }), + ), }, }, }, }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@openapi3/openapi3_version_test.go` around lines 193 - 217, The Post operation in TestJSONLookupWithWebhooks is missing a Responses value which will cause validation failures; update the Webhooks entry inside TestJSONLookupWithWebhooks so the Post Operation includes a minimal Responses (e.g., set Operation.Responses to a non-empty Responses instance such as NewResponses() or an explicit &Responses{...} with at least one response entry) so the constructed doc would also pass doc.Validate(ctx).openapi3/schema_const_test.go (2)
47-56: The "null const" test doesn't actually testconst: nullbehavior.The test sets
Const: nilwhich means "const not set" (per the comment), but the test name suggests it's testingconst: null. To properly test the JSON Schemaconst: nullcase where a value must equalnull, you would need to setConstIsSet: truealong withConst: nil.🧪 Proposed fix to properly test const:null
t.Run("null const", func(t *testing.T) { schema := &Schema{ - Type: &Types{"null"}, - Const: nil, + Type: &Types{"null"}, + Const: nil, + ConstIsSet: true, } - // nil const means "not set", so this should pass as empty schema + // const: null - value must be null err := schema.VisitJSON(nil) require.NoError(t, err) + + // Non-null value should fail + err = schema.VisitJSON("not null") + require.Error(t, err) + require.ErrorContains(t, err, "const") })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@openapi3/schema_const_test.go` around lines 47 - 56, The test "null const" incorrectly sets Const: nil (which currently means "not set") instead of representing an explicit const:null; update the test to set ConstIsSet: true alongside Const: nil on the Schema instance (i.e., in the test that constructs &Schema{ Type: &Types{"null"}, Const: nil, ... } add ConstIsSet: true) so that VisitJSON(nil) exercises the code path for an explicit const:null constraint and the test name matches the behavior being validated.
58-68: Object const comparison may fail with JSON-decoded values.The
visitConstOperationimplementation usesreflect.DeepEqualwhich is type-sensitive. This test passes because bothConstand the validated value are in-memory Go map literals with identical types. However, if the input were decoded from actual JSON (where all objects becomemap[string]any), type mismatches in nested values could cause unexpected failures. Consider adding a test case that validates JSON-decoded input against the const.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@openapi3/schema_const_test.go` around lines 58 - 68, Add a test that exercises const comparison with JSON-decoded input: create the schema.Const by unmarshaling a JSON byte blob (e.g. []byte(`{"key":"value"}`) into a map[string]any) and then call Schema.VisitJSON with a value decoded from JSON (not a Go literal) to ensure visitConstOperation and Schema.VisitJSON correctly compare JSON-decoded maps; reference the Schema type, its VisitJSON method, and the visitConstOperation behavior when adding this new subtest (e.g., t.Run("object const from json", ...)) so the test fails if reflect.DeepEqual type-sensitivity causes issues.openapi3/schema_validate_31_test.go (1)
10-96: Consider adding test cases forIf/Then/Elsesub-schema validation.The test comprehensively covers most OpenAPI 3.1 sub-schema locations but omits
If,Then, andElsewhich are also validated recursively (peropenapi3/schema.go:1491-1520). For completeness, consider adding:🧪 Proposed addition for conditional sub-schemas
t.Run("if with invalid sub-schema", func(t *testing.T) { schema := &Schema{ If: &SchemaRef{Value: invalidSchema}, } err := schema.Validate(ctx) require.Error(t, err, "should detect invalid sub-schema in if") }) t.Run("then with invalid sub-schema", func(t *testing.T) { schema := &Schema{ Then: &SchemaRef{Value: invalidSchema}, } err := schema.Validate(ctx) require.Error(t, err, "should detect invalid sub-schema in then") }) t.Run("else with invalid sub-schema", func(t *testing.T) { schema := &Schema{ Else: &SchemaRef{Value: invalidSchema}, } err := schema.Validate(ctx) require.Error(t, err, "should detect invalid sub-schema in else") })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@openapi3/schema_validate_31_test.go` around lines 10 - 96, Add three sub-tests to TestSchemaValidate31SubSchemas that exercise conditional subschemas: create schemas using If, Then, and Else set to &SchemaRef{Value: invalidSchema} and call schema.Validate(ctx) expecting an error; specifically add t.Run cases named "if with invalid sub-schema", "then with invalid sub-schema", and "else with invalid sub-schema" which each build a Schema with the respective field (If, Then, Else) pointing at invalidSchema and assert require.Error(t, err). This ensures the Schema.Validate method's recursive validation over If/Then/Else (see Schema.Validate) is covered.openapi3/schema_jsonschema_validator.go (3)
193-201: Silent fallback on compilation errors may hide issues.When
newJSONSchemaValidatorfails, the code silently falls back to the built-in validator. While this ensures validation continues, it could mask schema issues (like unresolved$refs mentioned in the PR objectives). Consider logging the compilation error at debug level or returning it wrapped in the fallback case.📝 Consider logging fallback reason
The fallback is intentional per the PR objectives, but adding observability would help users understand when the external validator couldn't be used:
func (schema *Schema) visitJSONWithJSONSchema(settings *schemaValidationSettings, value any) error { validator, err := newJSONSchemaValidator(schema) if err != nil { + // Log at debug level: compilation failed, falling back to built-in validator return schema.visitJSON(settings, value) } return validator.validate(value) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@openapi3/schema_jsonschema_validator.go` around lines 193 - 201, The fallback in visitJSONWithJSONSchema silently ignores errors from newJSONSchemaValidator; update visitJSONWithJSONSchema to capture the error returned by newJSONSchemaValidator and either log the compilation error at debug/trace level or return a wrapped error when falling back to schema.visitJSON, so callers have observability into why the JSON Schema validator wasn't used (referencing newJSONSchemaValidator, visitJSONWithJSONSchema, schema.visitJSON and validator.validate to locate the logic).
17-47: Schema compilation creates new validator per call - consider caching.Each call to
visitJSONWithJSONSchemacreates a newjsonSchemaValidatorby marshaling the schema to JSON, transforming it, and compiling it. For schemas validated repeatedly, this could impact performance. The PR mentions removing an unsafesync.Mapcache; consider a safer caching strategy if performance becomes an issue.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@openapi3/schema_jsonschema_validator.go` around lines 17 - 47, The newJSONSchemaValidator function currently marshals, transforms and compiles a schema on every call (used by visitJSONWithJSONSchema), causing repeated expensive work; introduce a thread-safe cache keyed by a stable schema identifier (e.g., the marshaled schema bytes or a computed hash) to store and reuse *jsonSchemaValidator instances instead of recompiling each time; implement the cache as a package-level map protected by sync.RWMutex (or a concurrent-safe structure), ensure newJSONSchemaValidator checks the cache first, returns a cached validator when present, and only performs marshal/transform/compile and stores the result under the lock when missing, and keep the existing function names (newJSONSchemaValidator and visitJSONWithJSONSchema) as points to integrate the cache.
156-191: Error path construction may produce malformed paths.In
formatValidationError, the path construction logic at lines 158-163 has edge cases:
- If
parentPathis non-empty andpathis "/", the result is justparentPath, which is correct- If
parentPathis empty andpathis "/", the result is empty string, losing the root indicatorThis could make error messages less clear for root-level validation failures.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@openapi3/schema_jsonschema_validator.go` around lines 156 - 191, The path construction in formatValidationError builds "path" from verr.InstanceLocation and then combines it with parentPath but loses the root indicator when parentPath is empty and path == "/", producing an empty path; fix by normalizing and joining paths: compute path = "/" + strings.Join(verr.InstanceLocation, "/"), then if parentPath == "" keep path (and if path == "/" keep it as "/"), otherwise join parentPath and the child path without producing double slashes (e.g., strip trailing '/' from parentPath then append child path unless child path == "/"), so that root-level errors still show "/" and composed paths are well-formed; update any uses of path (error message and recursive formatValidationError calls) accordingly so SchemaError.Reason contains the corrected path.openapi3/schema.go (1)
2832-2867: PatternProperties regex compilation on every validation call.Each validation call that checks
patternPropertiescompiles regexes at runtime (line 2837). Unlike thepatternfield which usescompiledPatternssync.Map cache,patternPropertiespatterns are compiled fresh each time. For schemas validated frequently, this could impact performance.⚡ Consider caching compiled patternProperties regexes
You could use the same
compiledPatternssync.Map pattern used for thepatternfield:for pattern, ppRef := range schema.PatternProperties { if ppRef == nil || ppRef.Value == nil { continue } - re, err := regexp.Compile(intoGoRegexp(pattern)) + goPattern := intoGoRegexp(pattern) + cpiface, _ := compiledPatterns.Load(goPattern) + re, _ := cpiface.(*regexp.Regexp) + if re == nil { + var err error + re, err = regexp.Compile(goPattern) + if err != nil { + // handle error... + } + compiledPatterns.Store(goPattern, re) + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@openapi3/schema.go` around lines 2832 - 2867, The patternProperties loop currently compiles regexes on every validation pass (regexp.Compile(intoGoRegexp(pattern))); change it to reuse the existing compiledPatterns sync.Map cache used for the pattern field: attempt a Load for a key derived from the pattern (e.g., "patternProperties:"+pattern) and, if missing, compile via regexp.Compile(intoGoRegexp(pattern)) and store the compiled *regexp.Regexp in compiledPatterns; keep the current behavior when settings.patternValidationDisabled is true, and ensure the rest of the logic around re.MatchString(k) and subsequent visitJSON calls (ppRef.Value.visitJSON) and SchemaError creation remains unchanged.openapi3/schema_jsonschema_validator_test.go (2)
51-55: MissingEnableJSONSchema2020()in object validation test.Unlike other tests in
TestJSONSchema2020Validator_Basic, this object validation test callsVisitJSONwithoutEnableJSONSchema2020(). This means it's testing the built-in validator, not the JSON Schema 2020-12 validator. If this is intentional to verify both paths work, consider adding a comment or testing both explicitly.🔧 Proposed fix: Add EnableJSONSchema2020() for consistency
err := schema.VisitJSON(map[string]any{ "name": "John", "age": 30, - }) + }, EnableJSONSchema2020()) require.NoError(t, err)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@openapi3/schema_jsonschema_validator_test.go` around lines 51 - 55, The object validation call is invoking schema.VisitJSON(...) without enabling the JSON Schema 2020-12 validator, so it exercises the built-in validator instead of the intended TestJSONSchema2020Validator_Basic path; update the test to construct/use the schema with EnableJSONSchema2020() before calling schema.VisitJSON (or add an explicit additional assertion/comment if you intend to test both validators) so the object validation runs against the JSON Schema 2020-12 validator.
389-418: Test function name suggests 2020-12 testing but tests built-in validator.
TestBuiltInValidatorStillWorksis named to test the built-in validator, but line 395-396 and 416 useEnableJSONSchema2020(). This creates mixed testing. Consider either:
- Removing
EnableJSONSchema2020()calls to truly test the built-in validator- Renaming the test to reflect what's actually being tested
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@openapi3/schema_jsonschema_validator_test.go` around lines 389 - 418, The test TestBuiltInValidatorStillWorks mixes built-in and 2020-12 validator usage by passing EnableJSONSchema2020() into schema.VisitJSON calls; make it consistent by removing the EnableJSONSchema2020() option from the two VisitJSON calls (so both string and object subtests exercise the built-in validator), ensuring the test name matches behavior and referencing Schema.VisitJSON, TestBuiltInValidatorStillWorks, and EnableJSONSchema2020 in your change.openapi3/openapi3.go (2)
163-171: Clarify the purpose of deletingoriginKeyfrom Webhooks.Line 169 deletes
originKeyfromx.Webhooks. This appears to be cleaning up internal metadata from the webhooks map, but it differs from the pattern used elsewhere (deleting fromExtensions). IfWebhooksisnil, this could panic.🛡️ Add nil check for safety
// OpenAPI 3.1 fields delete(x.Extensions, "webhooks") delete(x.Extensions, "jsonSchemaDialect") if len(x.Extensions) == 0 { x.Extensions = nil } - delete(x.Webhooks, originKey) + if x.Webhooks != nil { + delete(x.Webhooks, originKey) + } *doc = T(x)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@openapi3/openapi3.go` around lines 163 - 171, The code deletes originKey from x.Webhooks without a nil check which can panic if Webhooks is nil; update the cleanup to mirror the Extensions pattern by verifying x.Webhooks is non-nil (and non-empty if you prefer) before calling delete(x.Webhooks, originKey), and if after deletion x.Webhooks is empty set it to nil; refer to x.Webhooks and originKey in openapi3.go when making this safe deletion.
320-329: Consider validatingjsonSchemaDialectagainst known dialects.The current validation checks that
jsonSchemaDialectis a valid absolute URI with a scheme, which is correct per spec. However, you might consider adding a warning or informational log if the dialect is not a recognized JSON Schema draft (e.g.,https://json-schema.org/draft/2020-12/schema), as an unrecognized dialect could lead to unexpected validation behavior.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@openapi3/openapi3.go` around lines 320 - 329, The current OpenAPI 3.1 jsonSchemaDialect validation (the block that reads doc.JSONSchemaDialect, calls url.Parse and checks u.Scheme) should keep its existing absolute-URI checks but also compare doc.JSONSchemaDialect against a small whitelist of known JSON Schema dialect URIs (e.g., "https://json-schema.org/draft/2020-12/schema", "http://json-schema.org/draft-07/schema#", etc.); if the parsed value is not in that set, emit an informational warning (use the package's logger if available or fmt) noting the unrecognized dialect so callers are aware it may produce unexpected validation behavior. Ensure you reference doc.JSONSchemaDialect in the comparison and leave the existing url.Parse and u.Scheme error behavior unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/docs/openapi3.txt:
- Line 18: Replace the absolute phrase "100% backward compatibility" with a
qualified statement to avoid an absolute guarantee; locate the sentence
containing "100% backward compatibility" and change it to tempered wording such
as "maintains strong backward compatibility with OpenAPI 3.0" or "designed to be
largely backward-compatible with OpenAPI 3.0," optionally adding a short caveat
about potential edge-case differences.
- Around line 1985-1989: Update the EnableJSONSchema2020()
SchemaValidationOption docstring to state the fallback behavior: explain that
while enabling this option makes validation prefer the jsonschema library for
JSON Schema 2020-12 (enabling OpenAPI 3.1 features), if
compilation/initialization of jsonschema fails at runtime the system will
gracefully fall back to the built-in validator (and should emit a warning or log
entry); reference the EnableJSONSchema2020() name in the text so users know
where to expect this behavior.
- Around line 598-601: Fix the sentence and spec reference for ExclusiveBound:
rewrite the paragraph for clarity so it reads that ExclusiveBound represents
exclusiveMinimum/exclusiveMaximum and that the type changed between OpenAPI
versions — in OpenAPI 3.0 (JSON Schema Draft 4) it is a boolean that modifies
minimum/maximum, and in OpenAPI 3.1 (JSON Schema 2020-12) it is a number
representing the actual exclusive bound; update the text around the symbol
ExclusiveBound accordingly.
In `@openapi3/issue230_test.go`:
- Around line 492-513: Remove the two empty conditional blocks left in the test:
delete the no-op if statements that call doc30.IsOpenAPI3_1() and
doc31.IsOpenAPI3_1() (the empty "if doc30.IsOpenAPI3_1() { }" and "if
doc31.IsOpenAPI3_1() { }" blocks) inside the "automatic version detection and
configuration" test; if you intended to assert behavior instead, replace each
empty if with an appropriate require/assert call that checks the expected
boolean result of doc30.IsOpenAPI3_1() and doc31.IsOpenAPI3_1().
In `@openapi3/loader.go`:
- Around line 51-54: The Loader struct's anchorIndex and dynamicAnchorIndex maps
are not cleared between document loads, which can leak anchors across runs;
update the resetVisitedPathItemRefs() function to also reset/clear
Loader.anchorIndex and Loader.dynamicAnchorIndex (e.g., reassign empty maps or
delete entries) so both anchor indices are initialized per load; locate the
Loader type and ensure resetVisitedPathItemRefs() resets these maps in addition
to existing visited-path logic.
- Around line 385-393: When resolving an anchor via loader.anchorIndex in the
block that checks if schema, ok := loader.anchorIndex[fragment]; ok and sets
sr.Value = schema, also call the SchemaRef method setRefPath (e.g.,
sr.setRefPath(componentPath or the appropriate ref path) or otherwise set
sr.refPath) on the resolved *SchemaRef before returning componentDoc,
componentPath, nil so the refPath is populated consistently with other
resolution paths; update the branch that returns nil, nil, fmt.Errorf(...) only
after setting refPath when applicable.
In `@openapi3/origin_test.go`:
- Around line 439-441: The file has a stale/misplaced comment above
TestOrigin_ConstAndExamplesStripped that describes a different test
(TestOrigin_OriginExistsInProperties); update the comment to accurately describe
what TestOrigin_ConstAndExamplesStripped verifies (that originKey is stripped
from Const and Examples) or move the existing comment to precede
TestOrigin_OriginExistsInProperties; edit the comment text near the
TestOrigin_ConstAndExamplesStripped function to reflect the actual assertion and
ensure the original "verifies that loading fails when a specification contains a
property named '__origin__'" comment sits with
TestOrigin_OriginExistsInProperties.
In `@openapi3/schema_jsonschema_validator.go`:
- Around line 53-62: The current nullable handling mutates schema["type"] to
["null"] when nullable:true exists without a type, which incorrectly restricts
valid values; update the logic in the nullable branch that inspects schema (the
schema map, keys "nullable" and "type") so that: if schema["type"] is a string
convert to a []any containing the original type plus "null"; if schema["type"]
is an array ensure "null" is present (append only if missing); if there is no
schema["type"] do NOT set type to ["null"] — simply remove the "nullable" key
and leave the schema types unconstrained (or add "null" only when an explicit
type exists).
In `@openapi3/schema.go`:
- Around line 445-455: ExclusiveBound.MarshalJSON currently returns nil,nil when
eb.MarshalYAML() yields a nil value, which is invalid for JSON marshaling;
update ExclusiveBound.MarshalJSON to return a valid JSON representation instead
of nil (e.g., return []byte("null"), nil) or alternatively handle empty
ExclusiveBound at the caller so the field is omitted (ensure changes are made in
the ExclusiveBound.MarshalJSON function to stop returning nil,nil and return
valid JSON bytes or an error).
In `@openapi3/security_scheme.go`:
- Around line 175-176: The SecurityScheme validation currently accepts type
"mutualTLS" regardless of OpenAPI version; change validation so "mutualTLS" is
allowed only for OpenAPI 3.1+. To do this, propagate version context into
SecurityScheme.Validate (add a version parameter or context object) or perform
the gate in validateSecurityRequirement/Route.Spec where the document version is
available: detect if the document is < 3.1 and return a validation error when a
SecurityScheme with Type "mutualTLS" is present; update callers of
SecurityScheme.Validate accordingly (e.g., validateSecurityRequirement) to
supply the version/context and ensure error messages reference "mutualTLS" and
the required OpenAPI version.
---
Nitpick comments:
In `@openapi3/openapi3_version_test.go`:
- Line 11: Remove the package-level ctx variable and instead declare a local
context inside each test that needs it (e.g., add ctx := context.Background() at
the top of TestWebhooksField and inside subtests that call doc.Validate(ctx));
update any references to the package-level ctx to use the local ctx so tests do
not share or shadow a global context.
- Around line 193-217: The Post operation in TestJSONLookupWithWebhooks is
missing a Responses value which will cause validation failures; update the
Webhooks entry inside TestJSONLookupWithWebhooks so the Post Operation includes
a minimal Responses (e.g., set Operation.Responses to a non-empty Responses
instance such as NewResponses() or an explicit &Responses{...} with at least one
response entry) so the constructed doc would also pass doc.Validate(ctx).
In `@openapi3/openapi3.go`:
- Around line 163-171: The code deletes originKey from x.Webhooks without a nil
check which can panic if Webhooks is nil; update the cleanup to mirror the
Extensions pattern by verifying x.Webhooks is non-nil (and non-empty if you
prefer) before calling delete(x.Webhooks, originKey), and if after deletion
x.Webhooks is empty set it to nil; refer to x.Webhooks and originKey in
openapi3.go when making this safe deletion.
- Around line 320-329: The current OpenAPI 3.1 jsonSchemaDialect validation (the
block that reads doc.JSONSchemaDialect, calls url.Parse and checks u.Scheme)
should keep its existing absolute-URI checks but also compare
doc.JSONSchemaDialect against a small whitelist of known JSON Schema dialect
URIs (e.g., "https://json-schema.org/draft/2020-12/schema",
"http://json-schema.org/draft-07/schema#", etc.); if the parsed value is not in
that set, emit an informational warning (use the package's logger if available
or fmt) noting the unrecognized dialect so callers are aware it may produce
unexpected validation behavior. Ensure you reference doc.JSONSchemaDialect in
the comparison and leave the existing url.Parse and u.Scheme error behavior
unchanged.
In `@openapi3/schema_const_test.go`:
- Around line 47-56: The test "null const" incorrectly sets Const: nil (which
currently means "not set") instead of representing an explicit const:null;
update the test to set ConstIsSet: true alongside Const: nil on the Schema
instance (i.e., in the test that constructs &Schema{ Type: &Types{"null"},
Const: nil, ... } add ConstIsSet: true) so that VisitJSON(nil) exercises the
code path for an explicit const:null constraint and the test name matches the
behavior being validated.
- Around line 58-68: Add a test that exercises const comparison with
JSON-decoded input: create the schema.Const by unmarshaling a JSON byte blob
(e.g. []byte(`{"key":"value"}`) into a map[string]any) and then call
Schema.VisitJSON with a value decoded from JSON (not a Go literal) to ensure
visitConstOperation and Schema.VisitJSON correctly compare JSON-decoded maps;
reference the Schema type, its VisitJSON method, and the visitConstOperation
behavior when adding this new subtest (e.g., t.Run("object const from json",
...)) so the test fails if reflect.DeepEqual type-sensitivity causes issues.
In `@openapi3/schema_jsonschema_validator_test.go`:
- Around line 51-55: The object validation call is invoking
schema.VisitJSON(...) without enabling the JSON Schema 2020-12 validator, so it
exercises the built-in validator instead of the intended
TestJSONSchema2020Validator_Basic path; update the test to construct/use the
schema with EnableJSONSchema2020() before calling schema.VisitJSON (or add an
explicit additional assertion/comment if you intend to test both validators) so
the object validation runs against the JSON Schema 2020-12 validator.
- Around line 389-418: The test TestBuiltInValidatorStillWorks mixes built-in
and 2020-12 validator usage by passing EnableJSONSchema2020() into
schema.VisitJSON calls; make it consistent by removing the
EnableJSONSchema2020() option from the two VisitJSON calls (so both string and
object subtests exercise the built-in validator), ensuring the test name matches
behavior and referencing Schema.VisitJSON, TestBuiltInValidatorStillWorks, and
EnableJSONSchema2020 in your change.
In `@openapi3/schema_jsonschema_validator.go`:
- Around line 193-201: The fallback in visitJSONWithJSONSchema silently ignores
errors from newJSONSchemaValidator; update visitJSONWithJSONSchema to capture
the error returned by newJSONSchemaValidator and either log the compilation
error at debug/trace level or return a wrapped error when falling back to
schema.visitJSON, so callers have observability into why the JSON Schema
validator wasn't used (referencing newJSONSchemaValidator,
visitJSONWithJSONSchema, schema.visitJSON and validator.validate to locate the
logic).
- Around line 17-47: The newJSONSchemaValidator function currently marshals,
transforms and compiles a schema on every call (used by
visitJSONWithJSONSchema), causing repeated expensive work; introduce a
thread-safe cache keyed by a stable schema identifier (e.g., the marshaled
schema bytes or a computed hash) to store and reuse *jsonSchemaValidator
instances instead of recompiling each time; implement the cache as a
package-level map protected by sync.RWMutex (or a concurrent-safe structure),
ensure newJSONSchemaValidator checks the cache first, returns a cached validator
when present, and only performs marshal/transform/compile and stores the result
under the lock when missing, and keep the existing function names
(newJSONSchemaValidator and visitJSONWithJSONSchema) as points to integrate the
cache.
- Around line 156-191: The path construction in formatValidationError builds
"path" from verr.InstanceLocation and then combines it with parentPath but loses
the root indicator when parentPath is empty and path == "/", producing an empty
path; fix by normalizing and joining paths: compute path = "/" +
strings.Join(verr.InstanceLocation, "/"), then if parentPath == "" keep path
(and if path == "/" keep it as "/"), otherwise join parentPath and the child
path without producing double slashes (e.g., strip trailing '/' from parentPath
then append child path unless child path == "/"), so that root-level errors
still show "/" and composed paths are well-formed; update any uses of path
(error message and recursive formatValidationError calls) accordingly so
SchemaError.Reason contains the corrected path.
In `@openapi3/schema_validate_31_test.go`:
- Around line 10-96: Add three sub-tests to TestSchemaValidate31SubSchemas that
exercise conditional subschemas: create schemas using If, Then, and Else set to
&SchemaRef{Value: invalidSchema} and call schema.Validate(ctx) expecting an
error; specifically add t.Run cases named "if with invalid sub-schema", "then
with invalid sub-schema", and "else with invalid sub-schema" which each build a
Schema with the respective field (If, Then, Else) pointing at invalidSchema and
assert require.Error(t, err). This ensures the Schema.Validate method's
recursive validation over If/Then/Else (see Schema.Validate) is covered.
In `@openapi3/schema.go`:
- Around line 2832-2867: The patternProperties loop currently compiles regexes
on every validation pass (regexp.Compile(intoGoRegexp(pattern))); change it to
reuse the existing compiledPatterns sync.Map cache used for the pattern field:
attempt a Load for a key derived from the pattern (e.g.,
"patternProperties:"+pattern) and, if missing, compile via
regexp.Compile(intoGoRegexp(pattern)) and store the compiled *regexp.Regexp in
compiledPatterns; keep the current behavior when
settings.patternValidationDisabled is true, and ensure the rest of the logic
around re.MatchString(k) and subsequent visitJSON calls (ppRef.Value.visitJSON)
and SchemaError creation remains unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 5ba83605-300d-4872-8a4c-26f8c8b29267
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (35)
.github/docs/openapi3.txtREADME.mdcmd/validate/main.gogo.modopenapi2conv/openapi2_conv.goopenapi3/doc.goopenapi3/example_jsonschema2020_test.goopenapi3/example_validation.goopenapi3/info.goopenapi3/issue230_test.goopenapi3/license.goopenapi3/loader.goopenapi3/loader_31_conditional_test.goopenapi3/loader_31_schema_refs_test.goopenapi3/media_type.goopenapi3/openapi3.goopenapi3/openapi3_version_test.goopenapi3/origin_test.goopenapi3/refs.goopenapi3/refs.tmplopenapi3/schema.goopenapi3/schema_const_test.goopenapi3/schema_if_then_else_test.goopenapi3/schema_jsonschema_validator.goopenapi3/schema_jsonschema_validator_test.goopenapi3/schema_types_test.goopenapi3/schema_validate_31_test.goopenapi3/schema_validation_settings.goopenapi3/security_scheme.goopenapi3/server.goopenapi3/testdata/schema31_conditional.ymlopenapi3/testdata/schema31refs.ymlopenapi3/validation_options.goopenapi3filter/validate_request.goopenapi3filter/validate_response.go
| - JSON Schema dialect specification | ||
| - SPDX license identifiers | ||
|
|
||
| The implementation maintains 100% backward compatibility with OpenAPI 3.0. |
There was a problem hiding this comment.
Avoid absolute backward-compatibility guarantee wording
Line 18 states “100% backward compatibility,” which is too absolute for public docs and can become false with edge-case behavior changes. Prefer a qualified statement.
Suggested wording
-The implementation maintains 100% backward compatibility with OpenAPI 3.0.
+The implementation is designed to preserve backward compatibility with OpenAPI 3.0.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| The implementation maintains 100% backward compatibility with OpenAPI 3.0. | |
| The implementation is designed to preserve backward compatibility with OpenAPI 3.0. |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/docs/openapi3.txt at line 18, Replace the absolute phrase "100%
backward compatibility" with a qualified statement to avoid an absolute
guarantee; locate the sentence containing "100% backward compatibility" and
change it to tempered wording such as "maintains strong backward compatibility
with OpenAPI 3.0" or "designed to be largely backward-compatible with OpenAPI
3.0," optionally adding a short caveat about potential edge-case differences.
Comment on lines
+598
to
+601
| ExclusiveBound represents exclusiveMinimum/exclusiveMaximum which changed | ||
| type between OpenAPI versions. In OpenAPI 3.0 (JSON Schema draft-04): | ||
| boolean that modifies minimum/maximum In OpenAPI 3.1 (JSON Schema 2020-12): | ||
| number representing the actual exclusive bound |
There was a problem hiding this comment.
Fix ExclusiveBound spec reference and sentence structure
Lines 599-601 are currently grammatically broken, and the OpenAPI 3.0 schema basis is documented inaccurately as “draft-04”. Please correct both for spec accuracy and readability.
Suggested wording
- type between OpenAPI versions. In OpenAPI 3.0 (JSON Schema draft-04):
- boolean that modifies minimum/maximum In OpenAPI 3.1 (JSON Schema 2020-12):
- number representing the actual exclusive bound
+ type between OpenAPI versions.
+ In OpenAPI 3.0, it is a boolean modifier for minimum/maximum.
+ In OpenAPI 3.1 (JSON Schema 2020-12), it is a numeric exclusive bound.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| ExclusiveBound represents exclusiveMinimum/exclusiveMaximum which changed | |
| type between OpenAPI versions. In OpenAPI 3.0 (JSON Schema draft-04): | |
| boolean that modifies minimum/maximum In OpenAPI 3.1 (JSON Schema 2020-12): | |
| number representing the actual exclusive bound | |
| ExclusiveBound represents exclusiveMinimum/exclusiveMaximum which changed | |
| type between OpenAPI versions. | |
| In OpenAPI 3.0, it is a boolean modifier for minimum/maximum. | |
| In OpenAPI 3.1 (JSON Schema 2020-12), it is a numeric exclusive bound. |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/docs/openapi3.txt around lines 598 - 601, Fix the sentence and spec
reference for ExclusiveBound: rewrite the paragraph for clarity so it reads that
ExclusiveBound represents exclusiveMinimum/exclusiveMaximum and that the type
changed between OpenAPI versions — in OpenAPI 3.0 (JSON Schema Draft 4) it is a
boolean that modifies minimum/maximum, and in OpenAPI 3.1 (JSON Schema 2020-12)
it is a number representing the actual exclusive bound; update the text around
the symbol ExclusiveBound accordingly.
Comment on lines
+1985
to
+1989
| func EnableJSONSchema2020() SchemaValidationOption | ||
| EnableJSONSchema2020 enables JSON Schema 2020-12 compliant validation. | ||
| This enables support for OpenAPI 3.1 and JSON Schema 2020-12 features. | ||
| When enabled, validation uses the jsonschema library instead of the built-in | ||
| validator. |
There was a problem hiding this comment.
Document fallback behavior for JSON Schema 2020 validation
The current text says validation uses the jsonschema library “instead of” the built-in validator, but this misses the documented fallback path when schema compilation fails. This can mislead users about runtime behavior.
Suggested wording
func EnableJSONSchema2020() SchemaValidationOption
EnableJSONSchema2020 enables JSON Schema 2020-12 compliant validation.
This enables support for OpenAPI 3.1 and JSON Schema 2020-12 features.
- When enabled, validation uses the jsonschema library instead of the built-in
- validator.
+ When enabled, validation uses the jsonschema library and may fall back to
+ the built-in validator if JSON Schema compilation fails.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| func EnableJSONSchema2020() SchemaValidationOption | |
| EnableJSONSchema2020 enables JSON Schema 2020-12 compliant validation. | |
| This enables support for OpenAPI 3.1 and JSON Schema 2020-12 features. | |
| When enabled, validation uses the jsonschema library instead of the built-in | |
| validator. | |
| func EnableJSONSchema2020() SchemaValidationOption | |
| EnableJSONSchema2020 enables JSON Schema 2020-12 compliant validation. | |
| This enables support for OpenAPI 3.1 and JSON Schema 2020-12 features. | |
| When enabled, validation uses the jsonschema library and may fall back to | |
| the built-in validator if JSON Schema compilation fails. |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/docs/openapi3.txt around lines 1985 - 1989, Update the
EnableJSONSchema2020() SchemaValidationOption docstring to state the fallback
behavior: explain that while enabling this option makes validation prefer the
jsonschema library for JSON Schema 2020-12 (enabling OpenAPI 3.1 features), if
compilation/initialization of jsonschema fails at runtime the system will
gracefully fall back to the built-in validator (and should emit a warning or log
entry); reference the EnableJSONSchema2020() name in the text so users know
where to expect this behavior.
Comment on lines
+492
to
+513
| t.Run("automatic version detection and configuration", func(t *testing.T) { | ||
| // Simulate loading 3.0 document | ||
| spec30 := []byte(`{"openapi":"3.0.3","info":{"title":"Test","version":"1.0.0"},"paths":{}}`) | ||
| var doc30 openapi3.T | ||
| err := json.Unmarshal(spec30, &doc30) | ||
| require.NoError(t, err) | ||
|
|
||
| if doc30.IsOpenAPI3_1() { | ||
| } | ||
|
|
||
| // Simulate loading 3.1 document | ||
| spec31 := []byte(`{"openapi":"3.1.0","info":{"title":"Test","version":"1.0.0"},"paths":{}}`) | ||
| var doc31 openapi3.T | ||
| err = json.Unmarshal(spec31, &doc31) | ||
| require.NoError(t, err) | ||
|
|
||
| if doc31.IsOpenAPI3_1() { | ||
| } | ||
|
|
||
| // Cleanup | ||
| }) | ||
| } |
There was a problem hiding this comment.
Empty if blocks are dead code.
The if blocks at lines 499-501 and 508-510 are empty. They appear to be leftover scaffolding from development. Either add the intended logic or remove them.
🧹 Proposed fix: Remove empty if blocks
- if doc30.IsOpenAPI3_1() {
- }
-
// Simulate loading 3.1 document
spec31 := []byte(`{"openapi":"3.1.0","info":{"title":"Test","version":"1.0.0"},"paths":{}}`)
var doc31 openapi3.T
err = json.Unmarshal(spec31, &doc31)
require.NoError(t, err)
-
- if doc31.IsOpenAPI3_1() {
- }
-
- // Cleanup
})📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| t.Run("automatic version detection and configuration", func(t *testing.T) { | |
| // Simulate loading 3.0 document | |
| spec30 := []byte(`{"openapi":"3.0.3","info":{"title":"Test","version":"1.0.0"},"paths":{}}`) | |
| var doc30 openapi3.T | |
| err := json.Unmarshal(spec30, &doc30) | |
| require.NoError(t, err) | |
| if doc30.IsOpenAPI3_1() { | |
| } | |
| // Simulate loading 3.1 document | |
| spec31 := []byte(`{"openapi":"3.1.0","info":{"title":"Test","version":"1.0.0"},"paths":{}}`) | |
| var doc31 openapi3.T | |
| err = json.Unmarshal(spec31, &doc31) | |
| require.NoError(t, err) | |
| if doc31.IsOpenAPI3_1() { | |
| } | |
| // Cleanup | |
| }) | |
| } | |
| t.Run("automatic version detection and configuration", func(t *testing.T) { | |
| // Simulate loading 3.0 document | |
| spec30 := []byte(`{"openapi":"3.0.3","info":{"title":"Test","version":"1.0.0"},"paths":{}}`) | |
| var doc30 openapi3.T | |
| err := json.Unmarshal(spec30, &doc30) | |
| require.NoError(t, err) | |
| // Simulate loading 3.1 document | |
| spec31 := []byte(`{"openapi":"3.1.0","info":{"title":"Test","version":"1.0.0"},"paths":{}}`) | |
| var doc31 openapi3.T | |
| err = json.Unmarshal(spec31, &doc31) | |
| require.NoError(t, err) | |
| }) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@openapi3/issue230_test.go` around lines 492 - 513, Remove the two empty
conditional blocks left in the test: delete the no-op if statements that call
doc30.IsOpenAPI3_1() and doc31.IsOpenAPI3_1() (the empty "if
doc30.IsOpenAPI3_1() { }" and "if doc31.IsOpenAPI3_1() { }" blocks) inside the
"automatic version detection and configuration" test; if you intended to assert
behavior instead, replace each empty if with an appropriate require/assert call
that checks the expected boolean result of doc30.IsOpenAPI3_1() and
doc31.IsOpenAPI3_1().
Comment on lines
+51
to
54
|
|
||
| anchorIndex map[string]*Schema | ||
| dynamicAnchorIndex map[string]*Schema | ||
| } |
There was a problem hiding this comment.
Anchor indices added but never reset between loads.
The anchorIndex and dynamicAnchorIndex maps are added to the Loader struct but are not reset in resetVisitedPathItemRefs(). If a Loader instance is reused to load multiple documents, anchors from previous documents could persist and cause incorrect resolution.
🛠️ Proposed fix: Reset anchor indices
func (loader *Loader) resetVisitedPathItemRefs() {
loader.visitedPathItemRefs = make(map[string]struct{})
loader.visitedRefs = make(map[string]struct{})
loader.visitedPath = nil
loader.backtrack = make(map[string][]func(value any))
+ loader.anchorIndex = nil
+ loader.dynamicAnchorIndex = nil
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@openapi3/loader.go` around lines 51 - 54, The Loader struct's anchorIndex and
dynamicAnchorIndex maps are not cleared between document loads, which can leak
anchors across runs; update the resetVisitedPathItemRefs() function to also
reset/clear Loader.anchorIndex and Loader.dynamicAnchorIndex (e.g., reassign
empty maps or delete entries) so both anchor indices are initialized per load;
locate the Loader type and ensure resetVisitedPathItemRefs() resets these maps
in addition to existing visited-path logic.
Comment on lines
+385
to
393
| if loader.anchorIndex != nil { | ||
| if schema, ok := loader.anchorIndex[fragment]; ok { | ||
| if sr, ok := resolved.(*SchemaRef); ok { | ||
| sr.Value = schema | ||
| return componentDoc, componentPath, nil | ||
| } | ||
| } | ||
| } | ||
| return nil, nil, fmt.Errorf("expected fragment prefix '#/' in URI %q", ref) |
There was a problem hiding this comment.
Anchor resolution returns early without setting refPath.
When resolving a fragment via anchorIndex, the code sets sr.Value but doesn't call setRefPath on the SchemaRef. This inconsistency with other resolution paths could cause issues when the ref path is needed later (e.g., for error messages or further resolution).
🛠️ Proposed fix: Set refPath for anchor resolution
if loader.anchorIndex != nil {
if schema, ok := loader.anchorIndex[fragment]; ok {
if sr, ok := resolved.(*SchemaRef); ok {
sr.Value = schema
+ pathRef := copyURI(componentPath)
+ if pathRef == nil {
+ pathRef = new(url.URL)
+ }
+ pathRef.Fragment = fragment
+ sr.setRefPath(pathRef)
return componentDoc, componentPath, nil
}
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if loader.anchorIndex != nil { | |
| if schema, ok := loader.anchorIndex[fragment]; ok { | |
| if sr, ok := resolved.(*SchemaRef); ok { | |
| sr.Value = schema | |
| return componentDoc, componentPath, nil | |
| } | |
| } | |
| } | |
| return nil, nil, fmt.Errorf("expected fragment prefix '#/' in URI %q", ref) | |
| if loader.anchorIndex != nil { | |
| if schema, ok := loader.anchorIndex[fragment]; ok { | |
| if sr, ok := resolved.(*SchemaRef); ok { | |
| sr.Value = schema | |
| pathRef := copyURI(componentPath) | |
| if pathRef == nil { | |
| pathRef = new(url.URL) | |
| } | |
| pathRef.Fragment = fragment | |
| sr.setRefPath(pathRef) | |
| return componentDoc, componentPath, nil | |
| } | |
| } | |
| } | |
| return nil, nil, fmt.Errorf("expected fragment prefix '#/' in URI %q", ref) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@openapi3/loader.go` around lines 385 - 393, When resolving an anchor via
loader.anchorIndex in the block that checks if schema, ok :=
loader.anchorIndex[fragment]; ok and sets sr.Value = schema, also call the
SchemaRef method setRefPath (e.g., sr.setRefPath(componentPath or the
appropriate ref path) or otherwise set sr.refPath) on the resolved *SchemaRef
before returning componentDoc, componentPath, nil so the refPath is populated
consistently with other resolution paths; update the branch that returns nil,
nil, fmt.Errorf(...) only after setting refPath when applicable.
Comment on lines
439
to
+441
| // TestOrigin_OriginExistsInProperties verifies that loading fails when a specification | ||
| // contains a property named "__origin__", highlighting a limitation in the current implementation. | ||
| func TestOrigin_ConstAndExamplesStripped(t *testing.T) { |
There was a problem hiding this comment.
Misplaced or stale comment.
The comment on lines 439-440 states "verifies that loading fails when a specification contains a property named 'origin'" but the test TestOrigin_ConstAndExamplesStripped actually verifies that originKey is stripped from Const and Examples fields. The comment appears to belong to TestOrigin_OriginExistsInProperties at line 477.
Proposed fix
-// TestOrigin_OriginExistsInProperties verifies that loading fails when a specification
-// contains a property named "__origin__", highlighting a limitation in the current implementation.
+// TestOrigin_ConstAndExamplesStripped verifies that the __origin__ key injected by the YAML
+// origin-tracking loader is stripped from Const and Examples fields during unmarshaling.
func TestOrigin_ConstAndExamplesStripped(t *testing.T) {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| // TestOrigin_OriginExistsInProperties verifies that loading fails when a specification | |
| // contains a property named "__origin__", highlighting a limitation in the current implementation. | |
| func TestOrigin_ConstAndExamplesStripped(t *testing.T) { | |
| // TestOrigin_ConstAndExamplesStripped verifies that the __origin__ key injected by the YAML | |
| // origin-tracking loader is stripped from Const and Examples fields during unmarshaling. | |
| func TestOrigin_ConstAndExamplesStripped(t *testing.T) { |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@openapi3/origin_test.go` around lines 439 - 441, The file has a
stale/misplaced comment above TestOrigin_ConstAndExamplesStripped that describes
a different test (TestOrigin_OriginExistsInProperties); update the comment to
accurately describe what TestOrigin_ConstAndExamplesStripped verifies (that
originKey is stripped from Const and Examples) or move the existing comment to
precede TestOrigin_OriginExistsInProperties; edit the comment text near the
TestOrigin_ConstAndExamplesStripped function to reflect the actual assertion and
ensure the original "verifies that loading fails when a specification contains a
property named '__origin__'" comment sits with
TestOrigin_OriginExistsInProperties.
Comment on lines
+53
to
+62
| if nullable, ok := schema["nullable"].(bool); ok && nullable { | ||
| if typeVal, ok := schema["type"].(string); ok { | ||
| // Convert to type array with null (must be []any for jsonschema compiler) | ||
| schema["type"] = []any{typeVal, "null"} | ||
| } else if _, hasType := schema["type"]; !hasType { | ||
| // nullable: true without type - add "null" to allow null values | ||
| schema["type"] = []any{"null"} | ||
| } | ||
| delete(schema, "nullable") | ||
| } |
There was a problem hiding this comment.
Nullable handling for type-less schemas may be too restrictive.
When nullable: true is set without a type, the code sets type to ["null"]. This means only null values would be valid, but the OpenAPI 3.0 nullable: true semantics mean "the existing type(s) plus null". Without an explicit type, any type should be allowed, so setting type: ["null"] is overly restrictive.
🐛 Proposed fix
if nullable, ok := schema["nullable"].(bool); ok && nullable {
if typeVal, ok := schema["type"].(string); ok {
// Convert to type array with null (must be []any for jsonschema compiler)
schema["type"] = []any{typeVal, "null"}
- } else if _, hasType := schema["type"]; !hasType {
- // nullable: true without type - add "null" to allow null values
- schema["type"] = []any{"null"}
}
+ // If no type is set, don't add type constraint - nullable just means null is allowed
+ // alongside whatever values the schema permits
delete(schema, "nullable")
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if nullable, ok := schema["nullable"].(bool); ok && nullable { | |
| if typeVal, ok := schema["type"].(string); ok { | |
| // Convert to type array with null (must be []any for jsonschema compiler) | |
| schema["type"] = []any{typeVal, "null"} | |
| } else if _, hasType := schema["type"]; !hasType { | |
| // nullable: true without type - add "null" to allow null values | |
| schema["type"] = []any{"null"} | |
| } | |
| delete(schema, "nullable") | |
| } | |
| if nullable, ok := schema["nullable"].(bool); ok && nullable { | |
| if typeVal, ok := schema["type"].(string); ok { | |
| // Convert to type array with null (must be []any for jsonschema compiler) | |
| schema["type"] = []any{typeVal, "null"} | |
| } | |
| // If no type is set, don't add type constraint - nullable just means null is allowed | |
| // alongside whatever values the schema permits | |
| delete(schema, "nullable") | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@openapi3/schema_jsonschema_validator.go` around lines 53 - 62, The current
nullable handling mutates schema["type"] to ["null"] when nullable:true exists
without a type, which incorrectly restricts valid values; update the logic in
the nullable branch that inspects schema (the schema map, keys "nullable" and
"type") so that: if schema["type"] is a string convert to a []any containing the
original type plus "null"; if schema["type"] is an array ensure "null" is
present (append only if missing); if there is no schema["type"] do NOT set type
to ["null"] — simply remove the "nullable" key and leave the schema types
unconstrained (or add "null" only when an explicit type exists).
Comment on lines
+445
to
+455
| // MarshalJSON returns the JSON encoding of ExclusiveBound. | ||
| func (eb ExclusiveBound) MarshalJSON() ([]byte, error) { | ||
| x, err := eb.MarshalYAML() | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| if x == nil { | ||
| return nil, nil | ||
| } | ||
| return json.Marshal(x) | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify how json.Marshal handles ExclusiveBound when empty
cat << 'EOF' | go run -
package main
import (
"encoding/json"
"fmt"
)
type ExclusiveBound struct {
Bool *bool
Value *float64
}
func (eb ExclusiveBound) MarshalJSON() ([]byte, error) {
if eb.Value != nil {
return json.Marshal(*eb.Value)
}
if eb.Bool != nil {
return json.Marshal(*eb.Bool)
}
return nil, nil
}
type Schema struct {
ExclusiveMin ExclusiveBound `json:"exclusiveMinimum,omitempty"`
Type string `json:"type"`
}
func main() {
s := Schema{Type: "number"}
b, _ := json.Marshal(s)
fmt.Println(string(b))
}
EOFRepository: api7/kin-openapi
Length of output: 82
🏁 Script executed:
cd /tmp && cat > test_marshaler.go << 'EOF'
package main
import (
"encoding/json"
"fmt"
)
type ExclusiveBound struct {
Bool *bool
Value *float64
}
func (eb ExclusiveBound) MarshalJSON() ([]byte, error) {
if eb.Value != nil {
return json.Marshal(*eb.Value)
}
if eb.Bool != nil {
return json.Marshal(*eb.Bool)
}
return nil, nil
}
type Schema struct {
ExclusiveMin ExclusiveBound `json:"exclusiveMinimum,omitempty"`
Type string `json:"type"`
}
func main() {
s := Schema{Type: "number"}
b, _ := json.Marshal(s)
fmt.Println("Output:", string(b))
// Also test with non-nil value
val := 5.0
s2 := Schema{
ExclusiveMin: ExclusiveBound{Value: &val},
Type: "number",
}
b2, _ := json.Marshal(s2)
fmt.Println("With value:", string(b2))
}
EOF
go run test_marshaler.goRepository: api7/kin-openapi
Length of output: 119
🏁 Script executed:
# Check the actual ExclusiveBound type definition in the repo
rg -A 20 "type ExclusiveBound" openapi3/Repository: api7/kin-openapi
Length of output: 1100
🏁 Script executed:
cd /tmp && cat > test_marshaler2.go << 'EOF'
package main
import (
"encoding/json"
"fmt"
)
type ExclusiveBound struct {
Bool *bool
Value *float64
}
func (eb ExclusiveBound) MarshalJSON() ([]byte, error) {
if eb.Value != nil {
return json.Marshal(*eb.Value)
}
if eb.Bool != nil {
return json.Marshal(*eb.Bool)
}
return nil, nil
}
type Schema struct {
ExclusiveMin ExclusiveBound `json:"exclusiveMinimum,omitempty"`
Type string `json:"type"`
}
func main() {
s := Schema{Type: "number"}
b, _ := json.Marshal(s)
fmt.Printf("JSON bytes: %v\n", b)
fmt.Printf("JSON string: %q\n", string(b))
// Test if null is included
fmt.Printf("Contains null: %v\n", string(b) == `{"type":"number"}`)
fmt.Printf("Contains exclusiveMinimum: %v\n", string(b) == `{"exclusiveMinimum":null,"type":"number"}`)
}
EOF
go run test_marshaler2.goRepository: api7/kin-openapi
Length of output: 144
🏁 Script executed:
# Also check what MarshalJSON returns in the actual code
rg -A 15 "func \(eb ExclusiveBound\) MarshalJSON" openapi3/Repository: api7/kin-openapi
Length of output: 756
🏁 Script executed:
cd /tmp && cat > test_omitempty.go << 'EOF'
package main
import (
"encoding/json"
"fmt"
"log"
)
type ExclusiveBound struct {
Bool *bool
Value *float64
}
func (eb ExclusiveBound) MarshalJSON() ([]byte, error) {
if eb.Value != nil {
return json.Marshal(*eb.Value)
}
if eb.Bool != nil {
return json.Marshal(*eb.Bool)
}
return nil, nil
}
type SchemaWithOmitempty struct {
ExclusiveMin ExclusiveBound `json:"exclusiveMinimum,omitempty"`
Type string `json:"type"`
}
type SchemaNoOmitempty struct {
ExclusiveMin ExclusiveBound `json:"exclusiveMinimum"`
Type string `json:"type"`
}
func main() {
// Test with omitempty
s1 := SchemaWithOmitempty{Type: "number"}
b1, err := json.Marshal(s1)
if err != nil {
log.Fatal(err)
}
fmt.Println("With omitempty:")
fmt.Println(" Raw output:", b1)
fmt.Println(" String:", string(b1))
// Test without omitempty
s2 := SchemaNoOmitempty{Type: "number"}
b2, err := json.Marshal(s2)
if err != nil {
log.Fatal(err)
}
fmt.Println("Without omitempty:")
fmt.Println(" Raw output:", b2)
fmt.Println(" String:", string(b2))
}
EOF
go run test_omitempty.goRepository: api7/kin-openapi
Length of output: 184
🏁 Script executed:
cd /tmp && cat > test_correct_behavior.go << 'EOF'
package main
import (
"encoding/json"
"fmt"
)
type ExclusiveBound struct {
Bool *bool
Value *float64
}
func (eb ExclusiveBound) MarshalJSON() ([]byte, error) {
if eb.Value != nil {
return json.Marshal(*eb.Value)
}
if eb.Bool != nil {
return json.Marshal(*eb.Bool)
}
// Returning nil, nil causes error
return nil, nil
}
type ExclusiveBoundFixed struct {
Bool *bool
Value *float64
}
func (eb ExclusiveBoundFixed) MarshalJSON() ([]byte, error) {
if eb.Value != nil {
return json.Marshal(*eb.Value)
}
if eb.Bool != nil {
return json.Marshal(*eb.Bool)
}
// Correct way: return explicit null bytes
return []byte("null"), nil
}
type Schema struct {
ExclusiveMin ExclusiveBound `json:"exclusiveMinimum,omitempty"`
Type string `json:"type"`
}
type SchemaFixed struct {
ExclusiveMin ExclusiveBoundFixed `json:"exclusiveMinimum,omitempty"`
Type string `json:"type"`
}
func main() {
// Test with buggy version
s1 := Schema{Type: "number"}
b1, err := json.Marshal(s1)
fmt.Println("Buggy version (nil,nil):")
fmt.Println(" Error:", err)
fmt.Println(" Output:", string(b1))
// Test with fixed version
s2 := SchemaFixed{Type: "number"}
b2, err := json.Marshal(s2)
fmt.Println("Fixed version (null bytes):")
fmt.Println(" Error:", err)
fmt.Println(" Output:", string(b2))
}
EOF
go run test_correct_behavior.goRepository: api7/kin-openapi
Length of output: 290
🏁 Script executed:
cd /tmp && cat > test_proper_omitempty.go << 'EOF'
package main
import (
"encoding/json"
"fmt"
)
// Approach 1: Using a pointer to a custom type
type ExclusiveBoundPtr *float64
func (eb ExclusiveBoundPtr) MarshalJSON() ([]byte, error) {
if eb == nil {
return nil, nil // This should properly omit
}
return json.Marshal(*eb)
}
// Approach 2: Standard type without custom marshaler
type SchemaStandard struct {
ExclusiveMin *float64 `json:"exclusiveMinimum,omitempty"`
Type string `json:"type"`
}
type SchemaWithCustom struct {
ExclusiveMin ExclusiveBoundPtr `json:"exclusiveMinimum,omitempty"`
Type string `json:"type"`
}
func main() {
// Standard approach (no custom marshaler)
s1 := SchemaStandard{Type: "number"}
b1, _ := json.Marshal(s1)
fmt.Println("Standard *float64 with omitempty:")
fmt.Println(" Output:", string(b1))
// Custom marshaler approach with pointer
var s2 SchemaWithCustom
s2.Type = "number"
// s2.ExclusiveMin is nil (zero value)
b2, _ := json.Marshal(s2)
fmt.Println("Custom marshaler (nil) with omitempty:")
fmt.Println(" Output:", string(b2))
}
EOF
go run test_proper_omitempty.goRepository: api7/kin-openapi
Length of output: 186
🏁 Script executed:
# Search for uses of ExclusiveBound marshaling
rg "exclusiveMinimum|exclusiveMaximum" openapi3/ -A 2 -B 2 | head -100Repository: api7/kin-openapi
Length of output: 5566
🏁 Script executed:
# Look for test files related to schema marshaling
fd "test" openapi3/ --type f | head -10Repository: api7/kin-openapi
Length of output: 387
🏁 Script executed:
# Check if there are any JSON marshaling tests
rg "MarshalJSON" openapi3/schema_test.go -A 5 -B 2Repository: api7/kin-openapi
Length of output: 42
🏁 Script executed:
# Check if there are actual test failures or issues with this
rg "ExclusiveBound" openapi3/ -B 2 -A 2 | grep -i test | head -20Repository: api7/kin-openapi
Length of output: 735
🏁 Script executed:
# Look at how MarshalJSON is actually being called/tested
fd "schema" openapi3/ -type f -name "*test*"Repository: api7/kin-openapi
Length of output: 230
🏁 Script executed:
# Check the actual implementation around lines 445-455 more carefully
sed -n '440,460p' openapi3/schema.goRepository: api7/kin-openapi
Length of output: 529
MarshalJSON returning nil, nil causes a JSON marshaling error.
When ExclusiveBound.MarshalJSON() returns nil, nil, the JSON encoder fails with "unexpected end of JSON input" rather than properly handling the field. Returning nil, nil from a custom marshaler is invalid; the marshaler must return valid JSON bytes or an error. For proper behavior with omitempty, return []byte("null"), nil instead, though a better approach may be to handle empty ExclusiveBound at a higher level where the field can truly be omitted.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@openapi3/schema.go` around lines 445 - 455, ExclusiveBound.MarshalJSON
currently returns nil,nil when eb.MarshalYAML() yields a nil value, which is
invalid for JSON marshaling; update ExclusiveBound.MarshalJSON to return a valid
JSON representation instead of nil (e.g., return []byte("null"), nil) or
alternatively handle empty ExclusiveBound at the caller so the field is omitted
(ensure changes are made in the ExclusiveBound.MarshalJSON function to stop
returning nil,nil and return valid JSON bytes or an error).
Comment on lines
+175
to
+176
| case "mutualTLS": | ||
| // OpenAPI 3.1: mutualTLS has no additional required fields |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if validation context carries OpenAPI version information that could be used for version-gating
ast-grep --pattern $'func ($_ *SecurityScheme) Validate(ctx context.Context, $$$) $_ {
$$$
}'
# Check how other validation functions handle version-specific logic
rg -n "openapi.*3\.1|version.*3\.1" --type go -C3Repository: api7/kin-openapi
Length of output: 9131
🏁 Script executed:
# Check if the validation context or SecurityScheme carries version info
rg -n "Version|version" openapi3/security_scheme.go
# Check the T (OpenAPI) struct to see version field
rg -n "type T struct" -A 30 openapi3/openapi3.go | head -50
# Look for how version is determined/used in validation
rg -n "\.OpenAPI|specVersion|Version\(" -B2 -A2 --type goRepository: api7/kin-openapi
Length of output: 7453
🏁 Script executed:
# Search for examples of version-gated validation logic
rg -n "3\.1|3\.0|spec.*version|Version.*==" --type go -C2 | head -100Repository: api7/kin-openapi
Length of output: 4683
🏁 Script executed:
# Look for AuthenticationFunc implementations to see if they handle mutualTLS
fd -e go | xargs rg -l "AuthenticationFunc" | head -5Repository: api7/kin-openapi
Length of output: 253
🏁 Script executed:
# Check how AuthenticationFunc is implemented and used
cat -n openapi3filter/validation_handler.go | head -100Repository: api7/kin-openapi
Length of output: 3313
🏁 Script executed:
# Look for version checking in validation context or validation code
rg -n "Version|version|IsOpenAPI" openapi3filter/ --type go -B2 -A2 | head -80Repository: api7/kin-openapi
Length of output: 4169
🏁 Script executed:
# Check how the validation context is constructed and if version is passed
rg -n "WithValidationOptions|NewValidationContext|func.*ctx" openapi3filter/validate_request.go -B2 -A5 | head -100Repository: api7/kin-openapi
Length of output: 2257
🏁 Script executed:
# Check RequestValidationInput structure
rg -n "type RequestValidationInput struct" -A 15 openapi3filter/Repository: api7/kin-openapi
Length of output: 1153
🏁 Script executed:
# Get full ValidateSecurityRequirements and validateSecurityRequirement implementations
rg -n "func.*ValidateSecurityRequirement" -A 40 openapi3filter/validate_request.goRepository: api7/kin-openapi
Length of output: 1414
🏁 Script executed:
# Search for AuthenticationFunc implementations in tests
rg -n "func.*AuthenticationFunc|AuthenticationFunc.*func" -B 2 -A 10 openapi3filter/ | head -150Repository: api7/kin-openapi
Length of output: 4289
🏁 Script executed:
# Check what AuthenticationInput contains
rg -n "type AuthenticationInput struct" -A 10 openapi3filter/Repository: api7/kin-openapi
Length of output: 907
🏁 Script executed:
# Check the full validateSecurityRequirement implementation to see how it calls AuthenticationFunc
rg -n "func validateSecurityRequirement" -A 60 openapi3filter/validate_request.goRepository: api7/kin-openapi
Length of output: 2341
🏁 Script executed:
# Search for any version gating in security-related validation
rg -n "IsOpenAPI|Version|version.*security|security.*version" openapi3filter/ --type go -C 3Repository: api7/kin-openapi
Length of output: 4497
🏁 Script executed:
# Check the full SecurityScheme.Validate function with all cases
cat -n openapi3/security_scheme.go | sed -n '151,217p'Repository: api7/kin-openapi
Length of output: 2591
🏁 Script executed:
# Look for any tests that use mutualTLS in OpenAPI 3.0 documents
rg -n "mutualTLS" --type go -B 5 -A 5Repository: api7/kin-openapi
Length of output: 799
🏁 Script executed:
# Check if there are any tests creating OpenAPI 3.0 documents with mutualTLS to see current behavior
rg -n "openapi.*3\.0|mutualTLS" --type go | grep -A3 -B3 mutualTLSRepository: api7/kin-openapi
Length of output: 520
Consider version-gating mutualTLS to OpenAPI 3.1+ if strict spec compliance is desired.
The mutualTLS security scheme type is only valid in OpenAPI 3.1+. The current implementation accepts it regardless of document version, which means OpenAPI 3.0.x documents using mutualTLS would pass validation despite being spec-non-compliant.
However, this behavior is consistent with other security scheme types (apiKey, http, oauth2, openIdConnect) in the codebase, which also lack version gating at the SecurityScheme validation level. If strict version enforcement is desired, it would likely need to be applied across all scheme types rather than just mutualTLS.
Note that the validation context in SecurityScheme.Validate() doesn't directly carry version information, though the Route.Spec (OpenAPI document) is available higher in the call stack via validateSecurityRequirement(). Implementing version-gating would require refactoring the validation signature to pass version context down.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@openapi3/security_scheme.go` around lines 175 - 176, The SecurityScheme
validation currently accepts type "mutualTLS" regardless of OpenAPI version;
change validation so "mutualTLS" is allowed only for OpenAPI 3.1+. To do this,
propagate version context into SecurityScheme.Validate (add a version parameter
or context object) or perform the gate in validateSecurityRequirement/Route.Spec
where the document version is available: detect if the document is < 3.1 and
return a validation error when a SecurityScheme with Type "mutualTLS" is
present; update callers of SecurityScheme.Validate accordingly (e.g.,
validateSecurityRequirement) to supply the version/context and ensure error
messages reference "mutualTLS" and the required OpenAPI version.
There was a problem hiding this comment.
♻️ Duplicate comments (3)
.github/docs/openapi3.txt (3)
1986-1990:⚠️ Potential issue | 🟡 MinorDocument JSON Schema validator fallback behavior.
Lines 1989-1990 currently imply hard replacement of the built-in validator, but runtime behavior includes fallback when JSON Schema compilation fails.
Suggested edit
- When enabled, validation uses the jsonschema library instead of the built-in - validator. + When enabled, validation prefers the jsonschema library and may fall back + to the built-in validator if JSON Schema compilation fails.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/docs/openapi3.txt around lines 1986 - 1990, The docs for EnableJSONSchema2020 / SchemaValidationOption incorrectly state it strictly replaces the built-in validator; update the wording to document that enabling JSON Schema 2020-12 causes the system to attempt validation via the jsonschema library but will fall back to the built-in validator if JSON Schema compilation or initialization fails at runtime, and include a brief note about what kinds of errors trigger the fallback and that a warning/log is emitted when fallback occurs.
598-601:⚠️ Potential issue | 🟡 MinorFix
ExclusiveBoundparagraph grammar and OpenAPI 3.0 schema reference.Lines 598-601 are currently grammatically broken, and the 3.0 schema-basis text is inaccurate. This should be rewritten for clarity and spec accuracy.
Suggested edit
- ExclusiveBound represents exclusiveMinimum/exclusiveMaximum which changed - type between OpenAPI versions. In OpenAPI 3.0 (JSON Schema draft-04): - boolean that modifies minimum/maximum In OpenAPI 3.1 (JSON Schema 2020-12): - number representing the actual exclusive bound + ExclusiveBound represents exclusiveMinimum/exclusiveMaximum, whose type + changed between OpenAPI versions. + In OpenAPI 3.0, it is a boolean modifier for minimum/maximum. + In OpenAPI 3.1 (JSON Schema 2020-12), it is a numeric exclusive bound.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/docs/openapi3.txt around lines 598 - 601, Rewrite the "ExclusiveBound" paragraph to fix grammar and correct the schema reference: state that in OpenAPI 3.0 (based on JSON Schema draft-04) exclusiveMinimum/exclusiveMaximum are booleans that modify the adjacent minimum/maximum, and in OpenAPI 3.1 (based on JSON Schema 2020-12) exclusiveMinimum/exclusiveMaximum are numbers representing the actual exclusive bound; update the paragraph text for clarity and split into two clear sentences referencing ExclusiveBound, OpenAPI 3.0 (draft-04) and OpenAPI 3.1 (2020-12).
18-18:⚠️ Potential issue | 🟡 MinorAvoid absolute compatibility guarantee wording.
Line 18 uses “100% backward compatibility,” which is too absolute for public API docs and can become inaccurate with edge cases.
Suggested edit
-The implementation maintains 100% backward compatibility with OpenAPI 3.0. +The implementation is designed to preserve backward compatibility with OpenAPI 3.0.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/docs/openapi3.txt at line 18, Replace the absolute phrase "100% backward compatibility with OpenAPI 3.0" with non-absolute wording to avoid a guaranteed claim; locate the sentence containing the exact phrase "100% backward compatibility" and update it to a softer alternative such as "maintains strong backward compatibility with OpenAPI 3.0 in typical scenarios" or "aims to remain backward compatible with OpenAPI 3.0" and optionally add a short caveat about edge cases or potential differences.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In @.github/docs/openapi3.txt:
- Around line 1986-1990: The docs for EnableJSONSchema2020 /
SchemaValidationOption incorrectly state it strictly replaces the built-in
validator; update the wording to document that enabling JSON Schema 2020-12
causes the system to attempt validation via the jsonschema library but will fall
back to the built-in validator if JSON Schema compilation or initialization
fails at runtime, and include a brief note about what kinds of errors trigger
the fallback and that a warning/log is emitted when fallback occurs.
- Around line 598-601: Rewrite the "ExclusiveBound" paragraph to fix grammar and
correct the schema reference: state that in OpenAPI 3.0 (based on JSON Schema
draft-04) exclusiveMinimum/exclusiveMaximum are booleans that modify the
adjacent minimum/maximum, and in OpenAPI 3.1 (based on JSON Schema 2020-12)
exclusiveMinimum/exclusiveMaximum are numbers representing the actual exclusive
bound; update the paragraph text for clarity and split into two clear sentences
referencing ExclusiveBound, OpenAPI 3.0 (draft-04) and OpenAPI 3.1 (2020-12).
- Line 18: Replace the absolute phrase "100% backward compatibility with OpenAPI
3.0" with non-absolute wording to avoid a guaranteed claim; locate the sentence
containing the exact phrase "100% backward compatibility" and update it to a
softer alternative such as "maintains strong backward compatibility with OpenAPI
3.0 in typical scenarios" or "aims to remain backward compatible with OpenAPI
3.0" and optionally add a short caveat about edge cases or potential
differences.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3b9d7ff0-65e0-4a36-bc7f-023fc943a409
📒 Files selected for processing (1)
.github/docs/openapi3.txt
added 5 commits
March 26, 2026 17:46
There was a problem hiding this comment.
🧹 Nitpick comments (1)
openapi3/loader.go (1)
1108-1147: Consider documenting which sibling fields are intentionally supported.The
mergeSiblingFieldsfunction handles annotation siblings (description,title,default,readOnly,writeOnly,deprecated,example,examples,nullable) which aligns with JSON Schema's typical $ref sibling use cases. However, a brief comment explaining the rationale would clarify that structural/validation keywords are intentionally excluded.📝 Suggested documentation
func mergeSiblingFields(base *Schema, siblings map[string]any) *Schema { + // Merge annotation keywords that are commonly used as $ref siblings in OpenAPI 3.1. + // Structural/validation keywords are not merged as they would conflict with the referenced schema. merged := *base for k, v := range siblings {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@openapi3/loader.go` around lines 1108 - 1147, Add a brief explanatory comment above mergeSiblingFields describing that it intentionally only copies annotation siblings (description, title, default, readOnly, writeOnly, deprecated, example, examples, nullable) from the siblings map into a new Schema copy and deliberately excludes structural/validation keywords (e.g., type, properties, items, required, allOf, anyOf, etc.) so callers avoid merging validation into $ref targets; reference the function name mergeSiblingFields and the Schema type in the comment so readers know where this behavior is implemented and why.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@openapi3/loader.go`:
- Around line 1108-1147: Add a brief explanatory comment above
mergeSiblingFields describing that it intentionally only copies annotation
siblings (description, title, default, readOnly, writeOnly, deprecated, example,
examples, nullable) from the siblings map into a new Schema copy and
deliberately excludes structural/validation keywords (e.g., type, properties,
items, required, allOf, anyOf, etc.) so callers avoid merging validation into
$ref targets; reference the function name mergeSiblingFields and the Schema type
in the comment so readers know where this behavior is implemented and why.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c258e0a7-d287-4321-8292-646154794978
📒 Files selected for processing (7)
.github/docs/openapi3.txtopenapi3/components.goopenapi3/loader.goopenapi3/loader_test.goopenapi3/testdata/components-path-items.ymlopenapi3filter/validate_request_test.gorouters/gorillamux/router_test.go
✅ Files skipped from review due to trivial changes (1)
- openapi3/loader_test.go
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@openapi3filter/components_path_items_maingo_test.go`:
- Around line 35-53: The test builds a headers map with lowercase keys so
ValidateRequestBody's req.Header.Get("Content-Type") returns empty; canonicalize
header names using http.CanonicalHeaderKey before attaching them to the request
and assign as an http.Header (convert the map[string][]string to http.Header) so
httpReq.Header.Get("Content-Type") works as expected; update the block that
fills "headers" and the assignment to "httpReq.Header" (refer to headers,
httpReq, and ValidateRequestBody/req.Header.Get) to perform the canonicalization
and proper type conversion.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0c2ff51e-0767-4423-a8f0-4bd9be2a3a64
📒 Files selected for processing (2)
openapi3filter/components_path_items_maingo_test.goopenapi3filter/components_path_items_test.go
Comment on lines
+35
to
+53
| headersJSON := `{"content-type":"application/json"}` | ||
| h := make(map[string]interface{}) | ||
| err = json.Unmarshal([]byte(headersJSON), &h) | ||
| require.NoError(t, err) | ||
| headers := make(map[string][]string) | ||
| for k, v := range h { | ||
| switch val := v.(type) { | ||
| case string: | ||
| headers[k] = []string{val} | ||
| case []interface{}: | ||
| for _, item := range val { | ||
| headers[k] = append(headers[k], item.(string)) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| httpReq, _ := http.NewRequest(method, path, strings.NewReader(body)) | ||
| httpReq.Header = headers | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In Go net/http, if a request header map is assigned directly with key "content-type" (lowercase), will req.Header.Get("Content-Type") find it? Please cite docs for Header.Get and canonical MIME key behavior.
💡 Result:
No, req.Header.Get("Content-Type") will not find a header set by direct map assignment with the lowercase key "content-type". The net/http.Header.Get method canonicalizes the provided key using textproto.CanonicalMIMEHeaderKey (which turns "Content-Type" into "Content-Type"), then looks up that exact canonical key in the internal map. It assumes all stored keys are already in canonical form: "Get assumes that all keys are stored in canonical form. To use non-canonical keys, access the map directly." Direct map assignment req.Header["content-type"] = []string{"value"} stores the key as the lowercase "content-type", which does not match the canonical "Content-Type". Thus, Get returns "". Header methods like Set use CanonicalMIMEHeaderKey internally, storing canonical keys, so they work with Get. Canonical form for "content-type" is "Content-Type" (first letters uppercased, rest lowercase).
Citations:
- 1: net/http: Elaborate requirement for header key format golang/go#53140
- 2: https://ron-liu.medium.com/what-canonical-http-header-mean-in-golang-2e97f854316d
- 3: https://go.dev/src/net/http/header.go
- 4: https://stackoverflow.com/questions/65197129/cannot-get-http-header
- 5: https://go.dev/src/net/textproto/header.go
🏁 Script executed:
# First, find and inspect the test file
fd "components_path_items_maingo_test.go" -t fRepository: api7/kin-openapi
Length of output: 111
🏁 Script executed:
# Once located, read the specific lines mentioned (35-53)
cat -n openapi3filter/components_path_items_maingo_test.go | sed -n '30,60p'Repository: api7/kin-openapi
Length of output: 1349
🏁 Script executed:
# Also check the full context of the test function to understand what it's testing
cat -n openapi3filter/components_path_items_maingo_test.go | head -100Repository: api7/kin-openapi
Length of output: 3561
🏁 Script executed:
# Find and inspect the ValidateRequest function
fd "validate.*\.go" openapi3filter -t f | head -5Repository: api7/kin-openapi
Length of output: 264
🏁 Script executed:
# Search for Content-Type usage in the validation logic
rg "Content-Type|ContentType" openapi3filter --type go -A 3 -B 1 | head -50Repository: api7/kin-openapi
Length of output: 2636
🏁 Script executed:
# Check specifically in ValidateRequest or RequestValidationInput
rg "func.*ValidateRequest|ContentType" openapi3filter --type go -A 5 | head -80Repository: api7/kin-openapi
Length of output: 5127
🏁 Script executed:
# Check the validate_request.go file to see how it uses Content-Type
cat -n openapi3filter/validate_request.go | head -150Repository: api7/kin-openapi
Length of output: 5305
🏁 Script executed:
# Search for where Content-Type is read in ValidateRequestBody or similar functions
rg "Content-Type|req.Header.Get" openapi3filter/validate_request.go -A 2 -B 1 | head -40Repository: api7/kin-openapi
Length of output: 306
🏁 Script executed:
# Get more context around the ValidateRequestBody function to see the complete picture
rg "func ValidateRequestBody" openapi3filter -A 50 | head -80Repository: api7/kin-openapi
Length of output: 3065
🏁 Script executed:
# Also check what headerCT is defined as
rg "headerCT.*:=" openapi3filter -B 2 -A 2Repository: api7/kin-openapi
Length of output: 42
🏁 Script executed:
# Search for headerCT definition more broadly
rg "headerCT" openapi3filter --type go | head -10Repository: api7/kin-openapi
Length of output: 808
🏁 Script executed:
# Look at the full ValidateRequestBody function to find where it reads the Content-Type
cat -n openapi3filter/validate_request.go | sed -n '151,250p'Repository: api7/kin-openapi
Length of output: 4407
🏁 Script executed:
# Find where inputMIME is used in ValidateRequestBody
rg "inputMIME" openapi3filter/validate_request.go -A 5 -B 5Repository: api7/kin-openapi
Length of output: 560
🏁 Script executed:
# Also find where headerCT is defined as a constant
rg "const headerCT" openapi3filter --type goRepository: api7/kin-openapi
Length of output: 42
🏁 Script executed:
# Search for headerCT definition in all files
rg "headerCT\s*=" openapi3filter --type goRepository: api7/kin-openapi
Length of output: 149
🏁 Script executed:
# Also look at the beginning of validate_request.go more broadly
head -300 openapi3filter/validate_request.go | tail -150Repository: api7/kin-openapi
Length of output: 5128
Canonicalize parsed header keys before assigning to httpReq.Header.
At lines 35–53, storing the header with lowercase key "content-type" and directly assigning the map causes ValidateRequestBody to call req.Header.Get("Content-Type") and receive an empty string. This makes the test fail for missing Content-Type rather than the intended missing required field in the request body schema, weakening the assertion.
🔧 Proposed fix
import (
"context"
"encoding/json"
"net/http"
+ "net/textproto"
"strings"
"testing"
@@
- headers := make(map[string][]string)
+ headers := make(http.Header)
for k, v := range h {
+ key := textproto.CanonicalMIMEHeaderKey(k)
switch val := v.(type) {
case string:
- headers[k] = []string{val}
+ headers.Add(key, val)
case []interface{}:
for _, item := range val {
- headers[k] = append(headers[k], item.(string))
+ s, ok := item.(string)
+ require.True(t, ok, "header %q must contain string values", key)
+ headers.Add(key, s)
}
}
}
- httpReq, _ := http.NewRequest(method, path, strings.NewReader(body))
+ httpReq, err := http.NewRequest(method, path, strings.NewReader(body))
+ require.NoError(t, err)
httpReq.Header = headers📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| headersJSON := `{"content-type":"application/json"}` | |
| h := make(map[string]interface{}) | |
| err = json.Unmarshal([]byte(headersJSON), &h) | |
| require.NoError(t, err) | |
| headers := make(map[string][]string) | |
| for k, v := range h { | |
| switch val := v.(type) { | |
| case string: | |
| headers[k] = []string{val} | |
| case []interface{}: | |
| for _, item := range val { | |
| headers[k] = append(headers[k], item.(string)) | |
| } | |
| } | |
| } | |
| httpReq, _ := http.NewRequest(method, path, strings.NewReader(body)) | |
| httpReq.Header = headers | |
| headersJSON := `{"content-type":"application/json"}` | |
| h := make(map[string]interface{}) | |
| err = json.Unmarshal([]byte(headersJSON), &h) | |
| require.NoError(t, err) | |
| headers := make(http.Header) | |
| for k, v := range h { | |
| key := textproto.CanonicalMIMEHeaderKey(k) | |
| switch val := v.(type) { | |
| case string: | |
| headers.Add(key, val) | |
| case []interface{}: | |
| for _, item := range val { | |
| s, ok := item.(string) | |
| require.True(t, ok, "header %q must contain string values", key) | |
| headers.Add(key, s) | |
| } | |
| } | |
| } | |
| httpReq, err := http.NewRequest(method, path, strings.NewReader(body)) | |
| require.NoError(t, err) | |
| httpReq.Header = headers |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@openapi3filter/components_path_items_maingo_test.go` around lines 35 - 53,
The test builds a headers map with lowercase keys so ValidateRequestBody's
req.Header.Get("Content-Type") returns empty; canonicalize header names using
http.CanonicalHeaderKey before attaching them to the request and assign as an
http.Header (convert the map[string][]string to http.Header) so
httpReq.Header.Get("Content-Type") works as expected; update the block that
fills "headers" and the assignment to "httpReq.Header" (refer to headers,
httpReq, and ValidateRequestBody/req.Header.Get) to perform the canonicalization
and proper type conversion.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Cherry-pick upstream OpenAPI 3.1 support from kin-openapi and implement the scoped validation enhancements described in AlinsRan/kin-openapi#2.
Changes
Cherry-picked from upstream (15 commits)
Scoped Implementation (9 items from issue getkin#2)
visitJSONWithJSONSchemafalls back to built-in validator when JSON Schema compilation fails (e.g. unresolved cross-schema$ref) instead of swallowing errors$ref+ sibling merge$reffor OpenAPI 3.1 documents onlypatternPropertiesvisitJSONObjectwith regex matching, named property priority, and proper additionalProperties fallbackprefixItemsi < len(PrefixItems)validate againstPrefixItems[i], remaining againstItemsconst: nullfixConstIsSetfield distinguishes "const not set" from "const: null" via JSON unmarshalling detection; also works for programmatically-set const valuesuseJSONSchema2020guard) per JSON Schema 2020-12 specsync.Mapcache that caused stale validator reuse across GC cycles$anchorresolution$anchorvalues and resolves fragment references that don't start with/$dynamicAnchorindexing$dynamicAnchorvalues for future$dynamicRefresolutionBug fixes discovered during testing
[]stringto[]anyintransformOpenAPIToJSONSchema— the jsonschema compiler requires[]anyfor type arrays$refsibling merge scope: Only merges siblings for OpenAPI 3.1 documents, preserving 3.0 behavior where$refsiblings are ignoredsync.Mapcache keyed byunsafe.Pointer— pointer reuse after GC caused stale validator hitsFiles Modified
openapi3/schema.go—ConstIsSet,patternProperties,prefixItems, format guards,UnmarshalJSONopenapi3/schema_jsonschema_validator.go— fallback fix, nullable[]any, cache removalopenapi3/loader.go—$refsibling merge,$anchor/$dynamicAnchorindexing,mergeSiblingFields()openapi3/refs.go—extraSiblingfield onSchemaRef,UnmarshalJSONsibling captureTesting
go test ./...)go vet ./...cleanSummary by CodeRabbit
New Features
Breaking Changes
Documentation
Tests