-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
fix: avoid stack overflow errors when using heavily recursive types #1377
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
Changes from all commits
e2ade03
c56f541
361f406
f1987e4
699d072
0fe6cdc
16eac5b
88a4fa1
8de2f09
44b762f
e8027a5
4d0f651
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,4 @@ | ||
| package: issue1373 | ||
| generate: | ||
| models: true | ||
| output: issue.gen.go |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| package issue1373 | ||
|
|
||
| //go:generate go run github.com/oapi-codegen/oapi-codegen/v2/cmd/oapi-codegen --config=config.yaml spec.yaml |
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,32 @@ | ||
| openapi: 3.0.2 | ||
| info: | ||
| version: '0.0.1' | ||
| title: example | ||
| description: | | ||
| Make sure that recursive $ref in allOf are handled properly | ||
| paths: | ||
| /example: | ||
| get: | ||
| operationId: exampleGet | ||
| responses: | ||
| '200': | ||
| description: "OK" | ||
| content: | ||
| 'application/json': | ||
| schema: | ||
| $ref: '#/components/schemas/RecursiveObject' | ||
|
|
||
| components: | ||
| schemas: | ||
| RecursiveObject: | ||
| allOf: | ||
| - $ref: "#/components/schemas/NonRecursiveObject" | ||
| - $ref: "#/components/schemas/RecursiveObject" | ||
| - properties: | ||
| FieldInRecursive:: | ||
| type: string | ||
|
|
||
| NonRecursiveObject: | ||
| properties: | ||
| FieldInNonRecursive: | ||
| type: string |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -38,7 +38,12 @@ func mergeSchemas(allOf []*openapi3.SchemaRef, path []string) (Schema, error) { | |
| if err != nil { | ||
| return Schema{}, err | ||
| } | ||
| schema, err = mergeOpenapiSchemas(schema, oneOfSchema, true) | ||
|
|
||
| seenSchemaRef := make(map[string]bool) | ||
| if allOf[i].Ref != "" { | ||
| seenSchemaRef[allOf[i].Ref] = true | ||
| } | ||
| schema, err = mergeOpenapiSchemas(schema, oneOfSchema, true, seenSchemaRef) | ||
|
Comment on lines
+42
to
+46
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Concretely, a spec like the following would still crash: RecursiveObject:
allOf:
- $ref: "#/components/schemas/RecursiveObject" # self-ref is allOf[0]
- $ref: "#/components/schemas/NonRecursiveObject"Seeding seenTopLevel := make(map[string]bool)
if allOf[0].Ref != "" {
seenTopLevel[allOf[0].Ref] = true
}
for i := 1; i < n; i++ {
seenSchemaRef := make(map[string]bool)
for k, v := range seenTopLevel { seenSchemaRef[k] = v }
if allOf[i].Ref != "" {
seenSchemaRef[allOf[i].Ref] = true
seenTopLevel[allOf[i].Ref] = true
}
...
} |
||
| if err != nil { | ||
| return Schema{}, fmt.Errorf("error merging schemas for AllOf: %w", err) | ||
| } | ||
|
|
@@ -71,11 +76,17 @@ func valueWithPropagatedRef(ref *openapi3.SchemaRef) (openapi3.Schema, error) { | |
| return schema, nil | ||
| } | ||
|
|
||
| func mergeAllOf(allOf []*openapi3.SchemaRef) (openapi3.Schema, error) { | ||
| func mergeAllOf(allOf []*openapi3.SchemaRef, seenSchemaRef map[string]bool) (openapi3.Schema, error) { | ||
| var schema openapi3.Schema | ||
| for _, schemaRef := range allOf { | ||
| var err error | ||
| schema, err = mergeOpenapiSchemas(schema, *schemaRef.Value, true) | ||
| if schemaRef.Ref != "" && seenSchemaRef[schemaRef.Ref] { | ||
| continue | ||
| } | ||
| if schemaRef.Ref != "" { | ||
| seenSchemaRef[schemaRef.Ref] = true | ||
| } | ||
| schema, err = mergeOpenapiSchemas(schema, *schemaRef.Value, true, seenSchemaRef) | ||
| if err != nil { | ||
| return openapi3.Schema{}, fmt.Errorf("error merging schemas for AllOf: %w", err) | ||
| } | ||
|
|
@@ -85,7 +96,7 @@ func mergeAllOf(allOf []*openapi3.SchemaRef) (openapi3.Schema, error) { | |
|
|
||
| // mergeOpenapiSchemas merges two openAPI schemas and returns the schema | ||
| // all of whose fields are composed. | ||
| func mergeOpenapiSchemas(s1, s2 openapi3.Schema, allOf bool) (openapi3.Schema, error) { | ||
| func mergeOpenapiSchemas(s1, s2 openapi3.Schema, allOf bool, seenSchemaRef map[string]bool) (openapi3.Schema, error) { | ||
| var result openapi3.Schema | ||
|
|
||
| result.Extensions = make(map[string]any, len(s1.Extensions)+len(s2.Extensions)) | ||
|
|
@@ -100,15 +111,15 @@ func mergeOpenapiSchemas(s1, s2 openapi3.Schema, allOf bool) (openapi3.Schema, e | |
| var err error | ||
| if s1.AllOf != nil { | ||
| var merged openapi3.Schema | ||
| merged, err = mergeAllOf(s1.AllOf) | ||
| merged, err = mergeAllOf(s1.AllOf, seenSchemaRef) | ||
| if err != nil { | ||
| return openapi3.Schema{}, fmt.Errorf("error transitive merging AllOf on schema 1") | ||
| } | ||
| s1 = merged | ||
| } | ||
| if s2.AllOf != nil { | ||
| var merged openapi3.Schema | ||
| merged, err = mergeAllOf(s2.AllOf) | ||
| merged, err = mergeAllOf(s2.AllOf, seenSchemaRef) | ||
| if err != nil { | ||
| return openapi3.Schema{}, fmt.Errorf("error transitive merging AllOf on schema 2") | ||
| } | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.