Skip to content

fix: avoid stack overflow errors when using heavily recursive types#1377

Open
yoshikipom wants to merge 10 commits intooapi-codegen:mainfrom
yoshikipom:fix/recursive-error-allof
Open

fix: avoid stack overflow errors when using heavily recursive types#1377
yoshikipom wants to merge 10 commits intooapi-codegen:mainfrom
yoshikipom:fix/recursive-error-allof

Conversation

@yoshikipom
Copy link
Copy Markdown

Issue

#1373
Stack overflow error happens merge_schemas.go when allOf contains circular $ref.

Cause & How to fix

https://github.com/deepmap/oapi-codegen/blob/1f53862bcc64573d3d0c4c105c71a8143e7b1816/pkg/codegen/merge_schemas.go#L87
mergeOpenapiSchemas is called recursively until all items in allOf are consumed. (allOf of item in allOf are also explored.) If same item is in the path of the exploration, stack overflow can happen.

https://github.com/deepmap/oapi-codegen/blob/1f53862bcc64573d3d0c4c105c71a8143e7b1816/pkg/codegen/merge_schemas.go#L85-L86
This code flatten schemas and merge them to handle allOf in allOf.

My approach is just skip to merge same schema because that doesn't affect to the merge result.

https://github.com/deepmap/oapi-codegen/compare/master...yoshikipom:oapi-codegen:fix/recursive-error-allof?expand=1#diff-423bfd1d22f4994f4f0c03f76f15af8371bee0bc5da50efa3ab696e1a60d7d49R41-R43

Commit

  • Add test e2ade03
    • I confirmed this test failed correctly before changes in next commit.
  • Fix stack overflow by skipping to merge same object c56f541

Test

Before fix

$ go test ./internal/test/issues/issue-1373/issue_test.go     
runtime: goroutine stack exceeds 1000000000-byte limit
runtime: sp=0x140203785d0 stack=[0x14020378000, 0x14040378000]
fatal error: stack overflow

runtime stack: ...

After fix

$ go test ./internal/test/issues/issue-1373/issue_test.go
ok      command-line-arguments  0.308s

@jamietanna jamietanna requested a review from a team as a code owner September 20, 2024 11:26
@jamietanna jamietanna changed the title Fix stack overflow error in handling of allOf fix: avoid stack overflow errors when using heavily recursive types Sep 20, 2024
@jamietanna jamietanna added the bug Something isn't working label Sep 20, 2024
@jamietanna
Copy link
Copy Markdown
Member

Interestingly, this changes behaviour of an existing case:

 diff --git a/internal/test/all_of/v2/openapi.gen.go b/internal/test/all_of/v2/openapi.gen.go
index ab43034..618cec4 100644
--- a/internal/test/all_of/v2/openapi.gen.go
+++ b/internal/test/all_of/v2/openapi.gen.go
@@ -32,10 +32,10 @@ type PersonProperties struct {
 
 // PersonWithID defines model for PersonWithID.
 type PersonWithID struct {
-	FirstName          string `json:"FirstName"`
-	GovernmentIDNumber *int64 `json:"GovernmentIDNumber,omitempty"`
-	ID                 int64  `json:"ID"`
-	LastName           string `json:"LastName"`
+	FirstName          *string `json:"FirstName,omitempty"`
+	GovernmentIDNumber *int64  `json:"GovernmentIDNumber,omitempty"`
+	ID                 int64   `json:"ID"`
+	LastName           *string `json:"LastName,omitempty"`
 }

I need to confirm if that's expected

@jamietanna jamietanna enabled auto-merge (squash) September 20, 2024 11:44
@jamietanna jamietanna modified the milestones: v2.4.0, v2.5.0 Sep 20, 2024
@jamietanna jamietanna modified the milestones: v2.5.0, v2.6.0 Jul 15, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants