-
Notifications
You must be signed in to change notification settings - Fork 3.8k
fix(openapi): support kin-openapi exclusive bounds #7607
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Closed
llebihan
wants to merge
1
commit into
projectdiscovery:dev
from
LebtekOrg:upstream-kin-openapi-v0.145
Closed
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,62 @@ | ||
| package openapi | ||
|
|
||
| import ( | ||
| "errors" | ||
| "testing" | ||
|
|
||
| "github.com/getkin/kin-openapi/openapi3" | ||
| "github.com/stretchr/testify/require" | ||
| ) | ||
|
|
||
| func TestOpenAPIExampleExclusiveBounds(t *testing.T) { | ||
| tests := []struct { | ||
| name string | ||
| schema *openapi3.Schema | ||
| want interface{} | ||
| }{ | ||
| { | ||
| name: "OpenAPI 3.0 boolean exclusive minimum", | ||
| schema: openapi3.NewIntegerSchema().WithMin(0).WithExclusiveMin(true), | ||
| want: 1, | ||
| }, | ||
| { | ||
| name: "OpenAPI 3.0 boolean exclusive maximum", | ||
| schema: openapi3.NewIntegerSchema().WithMax(0).WithExclusiveMax(true), | ||
| want: -1, | ||
| }, | ||
| { | ||
| name: "OpenAPI 3.1 numeric exclusive minimum", | ||
| schema: openapi3.NewIntegerSchema().WithExclusiveMinValue(10), | ||
| want: 11, | ||
| }, | ||
| { | ||
| name: "OpenAPI 3.1 numeric exclusive maximum", | ||
| schema: openapi3.NewIntegerSchema().WithExclusiveMaxValue(-10), | ||
| want: -11, | ||
| }, | ||
| { | ||
| name: "OpenAPI 3.1 numeric exclusive range", | ||
| schema: openapi3.NewIntegerSchema(). | ||
| WithExclusiveMinValue(10). | ||
| WithExclusiveMaxValue(12), | ||
| want: 11, | ||
| }, | ||
| } | ||
|
|
||
| for _, test := range tests { | ||
| t.Run(test.name, func(t *testing.T) { | ||
| got, err := openAPIExample(test.schema, make(map[*openapi3.Schema]*cachedSchema)) | ||
| require.NoError(t, err) | ||
| require.Equal(t, test.want, got) | ||
| }) | ||
| } | ||
| } | ||
|
|
||
| func TestOpenAPIExampleRejectsEmptyExclusiveIntegerRange(t *testing.T) { | ||
| schema := openapi3.NewIntegerSchema(). | ||
| WithExclusiveMinValue(1). | ||
| WithExclusiveMaxValue(2) | ||
|
|
||
| _, err := openAPIExample(schema, make(map[*openapi3.Schema]*cachedSchema)) | ||
| require.True(t, errors.Is(err, ErrNoExample)) | ||
| } |
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Midpoint branches mis-handle narrow exclusive integer ranges — valid schemas incorrectly return
ErrNoExample.For
schema.Type=integer,Min=0with exclusive minimum,Max=1(inclusive) — i.e.{"type":"integer","minimum":0,"exclusiveMinimum":true,"maximum":1}— the only valid value is1, but the current logic returnsErrNoExample:belowMinimum && maximum != nil→value = (0+1)/2 = 0.5math.Trunc(0.5)→0minimumExclusive && 0 <= 0→ErrNoExampleThe same defect is symmetric for
aboveMaximum && minimum != nil(e.g.minimum=-2,exclusiveMaximumat-1, integer — valid answer-2is also rejected). Unlike the boundary-only branches (belowMinimum,aboveMaximum), the midpoint branches never apply theFloor+1/Ceil-1integer-exclusivity adjustment, nor do they fall back if the midpoint truncates onto an excluded boundary. This directly undermines the exclusive-bound support this PR is meant to add, and isn't covered by the added tests (which all use ranges wide enough that the midpoint truncation happens to land safely inside the bounds).🐛 Proposed fix: try the boundary-adjusted candidate first, fall back to midpoint only if it doesn't fit
switch { case belowMinimum && maximum != nil: - value = (*minimum + *maximum) / 2.0 + value = *minimum + if minimumExclusive { + if schema.Type.Is("integer") { + value = math.Floor(value) + 1 + } else { + value = math.Nextafter(value, math.Inf(1)) + } + } + if value > *maximum || (maximumExclusive && value >= *maximum) { + value = (*minimum + *maximum) / 2.0 + } case belowMinimum: value = *minimum if minimumExclusive { if schema.Type.Is("integer") { value = math.Floor(value) + 1 } else { value = math.Nextafter(value, math.Inf(1)) } } case aboveMaximum && minimum != nil: - value = (*minimum + *maximum) / 2.0 + value = *maximum + if maximumExclusive { + if schema.Type.Is("integer") { + value = math.Ceil(value) - 1 + } else { + value = math.Nextafter(value, math.Inf(-1)) + } + } + if value < *minimum || (minimumExclusive && value <= *minimum) { + value = (*minimum + *maximum) / 2.0 + } case aboveMaximum: value = *maximum if maximumExclusive { if schema.Type.Is("integer") { value = math.Ceil(value) - 1 } else { value = math.Nextafter(value, math.Inf(-1)) } } }Please also add a regression test for this narrow-range case in
examples_test.goonce fixed.📝 Committable suggestion
🤖 Prompt for AI Agents