Skip to content

Commit 54fbda2

Browse files
committed
Fix stack overflow on circular schema refs in clearSchemaRefs
Signed-off-by: Guan-Ming (Wesley) Chiu <105915352+guan404ming@users.noreply.github.com>
1 parent a0a39b6 commit 54fbda2

3 files changed

Lines changed: 130 additions & 17 deletions

File tree

utils/component/openapi_generator.go

Lines changed: 22 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -196,9 +196,9 @@ func getResolvedManifest(manifest string) (string, error) {
196196
if doc.Components == nil || len(doc.Components.Schemas) == 0 {
197197
return "", ErrNoSchemasFound
198198
}
199-
visited := make(map[*openapi3.SchemaRef]bool)
199+
stack := make(map[*openapi3.Schema]bool)
200200
for _, schemaRef := range doc.Components.Schemas {
201-
clearSchemaRefs(schemaRef, visited)
201+
clearSchemaRefs(schemaRef, stack)
202202
}
203203
resolved, err := json.Marshal(doc)
204204
if err != nil {
@@ -208,35 +208,43 @@ func getResolvedManifest(manifest string) (string, error) {
208208
}
209209

210210
// clearSchemaRefs recursively clears $ref strings on all nested SchemaRefs
211-
// so that json.Marshal outputs fully inlined schemas. The visited set
212-
// prevents infinite recursion on circular references.
213-
func clearSchemaRefs(sr *openapi3.SchemaRef, visited map[*openapi3.SchemaRef]bool) {
214-
if sr == nil || visited[sr] {
211+
// so that json.Marshal outputs fully inlined schemas. The stack set tracks
212+
// Schema values (not SchemaRef pointers) on the current recursion path to
213+
// detect circular references. kin-openapi resolves $refs by creating
214+
// different SchemaRef objects that share the same underlying Schema pointer,
215+
// so tracking by *Schema is necessary to catch all cycles.
216+
func clearSchemaRefs(sr *openapi3.SchemaRef, stack map[*openapi3.Schema]bool) {
217+
if sr == nil {
215218
return
216219
}
217-
visited[sr] = true
218220
sr.Ref = ""
219221
s := sr.Value
220222
if s == nil {
221223
return
222224
}
225+
if stack[s] {
226+
sr.Value = &openapi3.Schema{}
227+
return
228+
}
229+
stack[s] = true
223230
for _, child := range s.AllOf {
224-
clearSchemaRefs(child, visited)
231+
clearSchemaRefs(child, stack)
225232
}
226233
for _, child := range s.AnyOf {
227-
clearSchemaRefs(child, visited)
234+
clearSchemaRefs(child, stack)
228235
}
229236
for _, child := range s.OneOf {
230-
clearSchemaRefs(child, visited)
237+
clearSchemaRefs(child, stack)
231238
}
232-
clearSchemaRefs(s.Not, visited)
239+
clearSchemaRefs(s.Not, stack)
233240
if s.Items != nil {
234-
clearSchemaRefs(s.Items, visited)
241+
clearSchemaRefs(s.Items, stack)
235242
}
236243
for _, prop := range s.Properties {
237-
clearSchemaRefs(prop, visited)
244+
clearSchemaRefs(prop, stack)
238245
}
239246
if s.AdditionalProperties.Schema != nil {
240-
clearSchemaRefs(s.AdditionalProperties.Schema, visited)
247+
clearSchemaRefs(s.AdditionalProperties.Schema, stack)
241248
}
249+
delete(stack, s)
242250
}
Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
package component
2+
3+
import (
4+
"fmt"
5+
"io"
6+
"net/http"
7+
"testing"
8+
)
9+
10+
// TestGetResolvedManifest_RealKubernetesSpec downloads the actual Kubernetes
11+
// OpenAPI spec and runs it through getResolvedManifest to verify that circular
12+
// references (e.g. JSONSchemaProps) do not cause a stack overflow.
13+
func TestGetResolvedManifest_RealKubernetesSpec(t *testing.T) {
14+
if testing.Short() {
15+
t.Skip("skipping integration test in short mode")
16+
}
17+
18+
// apiextensions spec contains JSONSchemaProps which references itself circularly.
19+
const specURL = "https://raw.githubusercontent.com/kubernetes/kubernetes/release-1.32/api/openapi-spec/v3/apis__apiextensions.k8s.io__v1_openapi.json"
20+
resp, err := http.Get(specURL)
21+
if err != nil {
22+
t.Fatalf("failed to fetch kubernetes spec: %v", err)
23+
}
24+
defer resp.Body.Close()
25+
26+
body, err := io.ReadAll(resp.Body)
27+
if err != nil {
28+
t.Fatalf("failed to read response body: %v", err)
29+
}
30+
31+
out, err := getResolvedManifest(string(body))
32+
if err != nil {
33+
t.Fatalf("getResolvedManifest failed: %v", err)
34+
}
35+
36+
if len(out) == 0 {
37+
t.Fatal("expected non-empty output")
38+
}
39+
fmt.Printf("resolved manifest length: %d bytes\n", len(out))
40+
}

utils/component/openapi_generator_test.go

Lines changed: 68 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -169,6 +169,50 @@ components:
169169
wantErr: true,
170170
errSubstr: "",
171171
},
172+
{
173+
name: "Self-referencing schema like JSONSchemaProps does not stack overflow",
174+
input: `{
175+
"openapi": "3.0.0",
176+
"info": {"title": "k8s-like", "version": "1.0"},
177+
"paths": {},
178+
"components": {
179+
"schemas": {
180+
"JSONSchemaProps": {
181+
"type": "object",
182+
"properties": {
183+
"description": {"type": "string"},
184+
"properties": {
185+
"type": "object",
186+
"additionalProperties": {"$ref": "#/components/schemas/JSONSchemaProps"}
187+
},
188+
"items": {
189+
"$ref": "#/components/schemas/JSONSchemaPropsOrArray"
190+
},
191+
"not": {
192+
"$ref": "#/components/schemas/JSONSchemaProps"
193+
},
194+
"allOf": {
195+
"type": "array",
196+
"items": {"$ref": "#/components/schemas/JSONSchemaProps"}
197+
}
198+
}
199+
},
200+
"JSONSchemaPropsOrArray": {
201+
"type": "object",
202+
"properties": {
203+
"schema": {"$ref": "#/components/schemas/JSONSchemaProps"},
204+
"jsonSchemas": {
205+
"type": "array",
206+
"items": {"$ref": "#/components/schemas/JSONSchemaProps"}
207+
}
208+
}
209+
}
210+
}
211+
}
212+
}`,
213+
checkPath: "components.schemas.JSONSchemaProps.properties.description",
214+
wantType: "string",
215+
},
172216
}
173217

174218
for _, tt := range tests {
@@ -278,7 +322,7 @@ func TestClearSchemaRefs(t *testing.T) {
278322

279323
for _, tt := range tests {
280324
t.Run(tt.name, func(t *testing.T) {
281-
visited := make(map[*openapi3.SchemaRef]bool)
325+
visited := make(map[*openapi3.Schema]bool)
282326
clearSchemaRefs(tt.sr, visited)
283327
if tt.sr != nil && tt.sr.Ref != "" {
284328
t.Errorf("Ref = %q, want empty", tt.sr.Ref)
@@ -294,15 +338,36 @@ func TestClearSchemaRefs_Circular(t *testing.T) {
294338
a.Value.Properties = openapi3.Schemas{"b": b}
295339
b.Value.Properties = openapi3.Schemas{"a": a}
296340

297-
visited := make(map[*openapi3.SchemaRef]bool)
298-
clearSchemaRefs(a, visited) // must not hang or panic
341+
stack := make(map[*openapi3.Schema]bool)
342+
clearSchemaRefs(a, stack) // must not hang or panic
299343

300344
if a.Ref != "" {
301345
t.Errorf("a.Ref = %q, want empty", a.Ref)
302346
}
303347
if b.Ref != "" {
304348
t.Errorf("b.Ref = %q, want empty", b.Ref)
305349
}
350+
351+
// json.Marshal must not stack overflow on the cleared structure.
352+
if _, err := json.Marshal(a); err != nil {
353+
t.Fatalf("json.Marshal failed after clearing circular refs: %v", err)
354+
}
355+
}
356+
357+
func TestClearSchemaRefs_SelfReference(t *testing.T) {
358+
// Schema that references itself (like JSONSchemaProps).
359+
self := &openapi3.SchemaRef{Value: &openapi3.Schema{
360+
Type: &openapi3.Types{"object"},
361+
}}
362+
self.Value.Properties = openapi3.Schemas{"nested": self}
363+
364+
stack := make(map[*openapi3.Schema]bool)
365+
clearSchemaRefs(self, stack)
366+
367+
// The self-referencing property should be replaced, breaking the cycle.
368+
if _, err := json.Marshal(self); err != nil {
369+
t.Fatalf("json.Marshal failed on self-referencing schema: %v", err)
370+
}
306371
}
307372

308373
// navigatePath walks a dot-separated path through nested maps.

0 commit comments

Comments
 (0)