Skip to content

Commit 37f2087

Browse files
Anai-Guolocalai-org-maint-bot
Anai-Guo
and
localai-org-maint-bot
authored
fix(grammars): reject cyclic $ref in JSON-schema grammar to prevent stack-overflow crash (#11020) (#11041)
* fix(grammars): reject cyclic $ref in JSON-schema grammar to prevent stack-overflow crash JSONSchemaConverter.visit resolved $ref entries by recursively calling itself with no cycle detection. A client-supplied grammar_json_functions schema whose $defs contains a self- or mutually-referential $ref (e.g. {"A": {"$ref": "#/$defs/A"}}) made visit recurse until the goroutine stack was exhausted, producing a fatal "stack overflow" that kills the whole process rather than failing the single request. The schema is converted synchronously in the /v1/chat/completions handler before any backend call, so this is an unauthenticated remote crash. Fixes #11020. Track the $ref targets currently on the recursion stack and error out when one is re-entered, while popping after each descent so sibling (non-cyclic) reuse of the same $ref is still allowed. Signed-off-by: Tai An <antai12232931@outlook.com> * fix(grammars): add a bounded recursion depth and cover llama31 $ref cycles Addresses the review on #11041. The stack-set approach catches cyclic $ref chains, but a deeply nested yet acyclic client schema (thousands of nested arrays/objects) can still recurse through visit until the goroutine stack is exhausted, which is the same unauthenticated remote crash surface as #11020. - Add a bounded depth counter to JSONSchemaConverter.visit (incremented with a defer-based cleanup, capped at maxSchemaDepth = 256, far above any realistic schema) so an over-deep schema fails the request with an ordinary error instead of crashing the process. - Apply the same cyclic-$ref guard and depth bound to LLama31SchemaConverter.visit, the other production grammar entry point named in #11020, which previously had no cycle detection at all. - Regression tests: a deeply nested acyclic schema is rejected while a moderately nested one still builds, plus direct/indirect $ref cycle and depth tests for the llama31 converter. Signed-off-by: Tai An <antai12232931@outlook.com> * test(grammars): make llama31 cycle fixtures valid function-call shapes The two new llama31 $ref-cycle specs asserted on "cyclic $ref" but the converter requires each top-level oneOf alternative to carry its function-name property before descending, so both fixtures failed earlier with "no function name found in the schema" and never reached the cycle guard. Give each fixture a valid llama31 shape: construct the converter with NewLLama31SchemaConverter("function"), put "function": {"const": "test"} on the top-level alternative, and hang the cyclic $ref under an arguments property, so all 29 grammar specs pass and the assertions genuinely observe the cyclic $ref error. Signed-off-by: Tai An <antai12232931@outlook.com> --------- Signed-off-by: Tai An <antai12232931@outlook.com> Co-authored-by: localai-org-maint-bot <bot-opensource@localaisrl.com>
1 parent c5ae41a commit 37f2087

4 files changed

Lines changed: 198 additions & 10 deletions

File tree

‎pkg/functions/grammars/json_schema.go‎

Lines changed: 35 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,9 +10,25 @@ import (
1010
"strings"
1111
)
1212

13+
// maxSchemaDepth bounds how deeply visit may recurse into a client-supplied
14+
// schema. A cyclic $ref is caught by refsInProgress, but a deeply nested yet
15+
// acyclic schema (e.g. thousands of nested arrays/objects) could still recurse
16+
// until the goroutine stack is exhausted and crash the whole process. Rejecting
17+
// the request once this depth is exceeded turns that potential crash into an
18+
// ordinary per-request error. The limit is far above any realistic schema.
19+
const maxSchemaDepth = 256
20+
1321
type JSONSchemaConverter struct {
1422
propOrder map[string]int
1523
rules Rules
24+
// refsInProgress tracks the $ref targets currently on the recursion stack
25+
// so a self- or mutually-referential schema cannot recurse forever. It is
26+
// pushed before descending into a referenced schema and popped afterwards,
27+
// so sibling (non-cyclic) reuse of the same $ref is still allowed.
28+
refsInProgress map[string]bool
29+
// depth is the current recursion depth of visit, bounded by maxSchemaDepth
30+
// to guard against stack exhaustion on deeply nested acyclic schemas.
31+
depth int
1632
}
1733

1834
func NewJSONSchemaConverter(propOrder string) *JSONSchemaConverter {
@@ -26,8 +42,9 @@ func NewJSONSchemaConverter(propOrder string) *JSONSchemaConverter {
2642
rules["space"] = SPACE_RULE
2743

2844
return &JSONSchemaConverter{
29-
propOrder: propOrderMap,
30-
rules: rules,
45+
propOrder: propOrderMap,
46+
rules: rules,
47+
refsInProgress: make(map[string]bool),
3148
}
3249
}
3350

@@ -60,6 +77,11 @@ func (sc *JSONSchemaConverter) addRule(name, rule string) string {
6077
}
6178

6279
func (sc *JSONSchemaConverter) visit(schema map[string]any, name string, rootSchema map[string]any) (string, error) {
80+
sc.depth++
81+
defer func() { sc.depth-- }()
82+
if sc.depth > maxSchemaDepth {
83+
return "", fmt.Errorf("schema nesting exceeds maximum depth %d while building grammar", maxSchemaDepth)
84+
}
6385
st, existType := schema["type"]
6486
var schemaType string
6587
var schemaTypes []string
@@ -115,11 +137,21 @@ func (sc *JSONSchemaConverter) visit(schema map[string]any, name string, rootSch
115137
rule := strings.Join(alternatives, " | ")
116138
return sc.addRule(ruleName, rule), nil
117139
} else if ref, exists := schema["$ref"].(string); exists {
140+
// A client-supplied schema may contain a cyclic $ref (e.g. a $def that
141+
// references itself directly or through a chain). Without this guard the
142+
// recursion below never terminates and exhausts the goroutine stack,
143+
// crashing the whole process rather than just failing the request.
144+
if sc.refsInProgress[ref] {
145+
return "", fmt.Errorf("cyclic $ref detected while building grammar: %s", ref)
146+
}
118147
referencedSchema, err := sc.resolveReference(ref, rootSchema)
119148
if err != nil {
120149
return "", err
121150
}
122-
return sc.visit(referencedSchema, name, rootSchema)
151+
sc.refsInProgress[ref] = true
152+
result, err := sc.visit(referencedSchema, name, rootSchema)
153+
delete(sc.refsInProgress, ref)
154+
return result, err
123155
} else if constVal, exists := schema["const"]; exists {
124156
literal, err := sc.formatLiteral((constVal))
125157
if err != nil {

‎pkg/functions/grammars/json_schema_test.go‎

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -605,3 +605,73 @@ var _ = Describe("JSON schema property ordering (issue #10052)", func() {
605605
Expect(keyIndex(grammar, "arguments")).To(BeNumerically("<", keyIndex(grammar, "aaa_unlisted")))
606606
})
607607
})
608+
609+
var _ = Describe("JSON schema grammar cyclic $ref handling", func() {
610+
// A client-supplied grammar_json_functions schema with a cyclic $ref used to
611+
// recurse until the goroutine stack overflowed, crashing the whole process
612+
// instead of failing the single request. The converter must now return an
613+
// error for such schemas rather than recursing forever.
614+
It("returns an error for a directly self-referential $ref", func() {
615+
const schema = `{
616+
"$defs": {"A": {"$ref": "#/$defs/A"}},
617+
"oneOf": [{"type": "object", "properties": {"x": {"$ref": "#/$defs/A"}}}]
618+
}`
619+
_, err := NewJSONSchemaConverter("").GrammarFromBytes([]byte(schema))
620+
Expect(err).To(HaveOccurred())
621+
Expect(err.Error()).To(ContainSubstring("cyclic $ref"))
622+
})
623+
624+
It("returns an error for an indirect $ref cycle (A -> B -> A)", func() {
625+
const schema = `{
626+
"$defs": {
627+
"A": {"type": "object", "properties": {"b": {"$ref": "#/$defs/B"}}},
628+
"B": {"type": "object", "properties": {"a": {"$ref": "#/$defs/A"}}}
629+
},
630+
"$ref": "#/$defs/A"
631+
}`
632+
_, err := NewJSONSchemaConverter("").GrammarFromBytes([]byte(schema))
633+
Expect(err).To(HaveOccurred())
634+
Expect(err.Error()).To(ContainSubstring("cyclic $ref"))
635+
})
636+
637+
It("still resolves a non-cyclic $ref reused by sibling properties", func() {
638+
const schema = `{
639+
"$defs": {"Leaf": {"type": "string"}},
640+
"type": "object",
641+
"properties": {
642+
"x": {"$ref": "#/$defs/Leaf"},
643+
"y": {"$ref": "#/$defs/Leaf"}
644+
}
645+
}`
646+
grammar, err := NewJSONSchemaConverter("").GrammarFromBytes([]byte(schema))
647+
Expect(err).To(BeNil())
648+
Expect(grammar).ToNot(BeEmpty())
649+
})
650+
651+
// A deeply nested but acyclic schema has no $ref cycle to catch, yet it can
652+
// still recurse deeply enough to exhaust the goroutine stack. The bounded
653+
// depth counter must reject it with an ordinary error instead of crashing.
654+
It("returns an error for a schema nested deeper than the depth limit", func() {
655+
// Wrap a string leaf in enough nested "array"/"items" layers to exceed
656+
// maxSchemaDepth; each layer is one visit() recursion.
657+
const layers = 400
658+
schema := `{"type": "string"}`
659+
for i := 0; i < layers; i++ {
660+
schema = `{"type": "array", "items": ` + schema + `}`
661+
}
662+
_, err := NewJSONSchemaConverter("").GrammarFromBytes([]byte(schema))
663+
Expect(err).To(HaveOccurred())
664+
Expect(err.Error()).To(ContainSubstring("maximum depth"))
665+
})
666+
667+
It("still builds a grammar for a moderately nested acyclic schema", func() {
668+
const layers = 32
669+
schema := `{"type": "string"}`
670+
for i := 0; i < layers; i++ {
671+
schema = `{"type": "array", "items": ` + schema + `}`
672+
}
673+
grammar, err := NewJSONSchemaConverter("").GrammarFromBytes([]byte(schema))
674+
Expect(err).To(BeNil())
675+
Expect(grammar).ToNot(BeEmpty())
676+
})
677+
})

‎pkg/functions/grammars/llama31_schema.go‎

Lines changed: 26 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,14 @@ import (
1212
type LLama31SchemaConverter struct {
1313
fnName string
1414
rules Rules
15+
// refsInProgress tracks the $ref targets currently on the recursion stack so
16+
// a self- or mutually-referential schema cannot recurse forever, mirroring
17+
// the guard in JSONSchemaConverter. This is a production entry point named
18+
// in the same crash report (#11020).
19+
refsInProgress map[string]bool
20+
// depth is the current recursion depth of visit, bounded by maxSchemaDepth
21+
// to guard against stack exhaustion on deeply nested acyclic schemas.
22+
depth int
1523
}
1624

1725
func NewLLama31SchemaConverter(fnName string) *LLama31SchemaConverter {
@@ -22,8 +30,9 @@ func NewLLama31SchemaConverter(fnName string) *LLama31SchemaConverter {
2230
}
2331

2432
return &LLama31SchemaConverter{
25-
rules: rules,
26-
fnName: fnName,
33+
rules: rules,
34+
fnName: fnName,
35+
refsInProgress: make(map[string]bool),
2736
}
2837
}
2938

@@ -74,6 +83,11 @@ func (sc *LLama31SchemaConverter) addRule(name, rule string) string {
7483
}
7584

7685
func (sc *LLama31SchemaConverter) visit(schema map[string]any, name string, rootSchema map[string]any) (string, error) {
86+
sc.depth++
87+
defer func() { sc.depth-- }()
88+
if sc.depth > maxSchemaDepth {
89+
return "", fmt.Errorf("schema nesting exceeds maximum depth %d while building grammar", maxSchemaDepth)
90+
}
7791
st, existType := schema["type"]
7892
var schemaType string
7993
if existType {
@@ -111,11 +125,20 @@ func (sc *LLama31SchemaConverter) visit(schema map[string]any, name string, root
111125
rule := strings.Join(alternatives, " | ")
112126
return sc.addRule(ruleName, rule), nil
113127
} else if ref, exists := schema["$ref"].(string); exists {
128+
// Guard against cyclic $ref chains that would otherwise recurse until
129+
// the goroutine stack is exhausted, crashing the whole process rather
130+
// than failing the single request (#11020).
131+
if sc.refsInProgress[ref] {
132+
return "", fmt.Errorf("cyclic $ref detected while building grammar: %s", ref)
133+
}
114134
referencedSchema, err := sc.resolveReference(ref, rootSchema)
115135
if err != nil {
116136
return "", err
117137
}
118-
return sc.visit(referencedSchema, name, rootSchema)
138+
sc.refsInProgress[ref] = true
139+
result, err := sc.visit(referencedSchema, name, rootSchema)
140+
delete(sc.refsInProgress, ref)
141+
return result, err
119142
} else if constVal, exists := schema["const"]; exists {
120143

121144
literal, err := sc.formatLiteral((constVal))

‎pkg/functions/grammars/llama31_schema_test.go‎

Lines changed: 67 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -43,8 +43,8 @@ const (
4343
// <function=example_function_name>{{"example_name": "example_value"}}</function>
4444
testllama31inputResult1 = `root-0-function ::= "create_event"
4545
freestring ::= (
46-
[^"\\] |
47-
"\\" (["\\/bfnrt] | "u" [0-9a-fA-F] [0-9a-fA-F] [0-9a-fA-F] [0-9a-fA-F])
46+
[^"\] |
47+
"\\" (["\/bfnrt] | "u" [0-9a-fA-F] [0-9a-fA-F] [0-9a-fA-F] [0-9a-fA-F])
4848
)* space
4949
root-0 ::= "<function=" root-0-function ">{" root-0-arguments "}</function>"
5050
root-1-arguments ::= "{" space "\"query\"" space ":" space string "}" space
@@ -53,8 +53,8 @@ space ::= " "?
5353
root-0-arguments ::= "{" space "\"date\"" space ":" space string "," space "\"time\"" space ":" space string "," space "\"title\"" space ":" space string "}" space
5454
root-1 ::= "<function=" root-1-function ">{" root-1-arguments "}</function>"
5555
string ::= "\"" (
56-
[^"\\] |
57-
"\\" (["\\/bfnrt] | "u" [0-9a-fA-F] [0-9a-fA-F] [0-9a-fA-F] [0-9a-fA-F])
56+
[^"\] |
57+
"\\" (["\/bfnrt] | "u" [0-9a-fA-F] [0-9a-fA-F] [0-9a-fA-F] [0-9a-fA-F])
5858
)* "\"" space
5959
root-1-function ::= "search"`
6060
)
@@ -74,3 +74,66 @@ var _ = Describe("JSON schema grammar tests", func() {
7474
})
7575
})
7676
})
77+
78+
var _ = Describe("LLama31 schema grammar cyclic $ref and depth handling", func() {
79+
// LLama31SchemaConverter.visit is another production entry point named in
80+
// the crash report (#11020). A cyclic $ref or a deeply nested acyclic schema
81+
// must fail the request rather than recurse until the stack is exhausted.
82+
//
83+
// The llama31 converter expects each top-level oneOf alternative to carry
84+
// its configured function-name property before it descends into arguments,
85+
// so the fixtures below use a valid function-call shape and hang the cyclic
86+
// $ref under `arguments` to make sure the cycle guard is what trips.
87+
It("returns an error for a directly self-referential $ref", func() {
88+
const schema = `{
89+
"$defs": {"A": {"$ref": "#/$defs/A"}},
90+
"oneOf": [
91+
{
92+
"type": "object",
93+
"properties": {
94+
"function": {"const": "test"},
95+
"arguments": {
96+
"type": "object",
97+
"properties": {"x": {"$ref": "#/$defs/A"}}
98+
}
99+
}
100+
}
101+
]
102+
}`
103+
_, err := NewLLama31SchemaConverter("function").GrammarFromBytes([]byte(schema))
104+
Expect(err).To(HaveOccurred())
105+
Expect(err.Error()).To(ContainSubstring("cyclic $ref"))
106+
})
107+
108+
It("returns an error for an indirect $ref cycle (A -> B -> A)", func() {
109+
const schema = `{
110+
"$defs": {
111+
"A": {"type": "object", "properties": {"b": {"$ref": "#/$defs/B"}}},
112+
"B": {"type": "object", "properties": {"a": {"$ref": "#/$defs/A"}}}
113+
},
114+
"oneOf": [
115+
{
116+
"type": "object",
117+
"properties": {
118+
"function": {"const": "test"},
119+
"arguments": {"$ref": "#/$defs/A"}
120+
}
121+
}
122+
]
123+
}`
124+
_, err := NewLLama31SchemaConverter("function").GrammarFromBytes([]byte(schema))
125+
Expect(err).To(HaveOccurred())
126+
Expect(err.Error()).To(ContainSubstring("cyclic $ref"))
127+
})
128+
129+
It("returns an error for a schema nested deeper than the depth limit", func() {
130+
const layers = 400
131+
schema := `{"type": "string"}`
132+
for i := 0; i < layers; i++ {
133+
schema = `{"type": "array", "items": ` + schema + `}`
134+
}
135+
_, err := NewLLama31SchemaConverter("").GrammarFromBytes([]byte(schema))
136+
Expect(err).To(HaveOccurred())
137+
Expect(err.Error()).To(ContainSubstring("maximum depth"))
138+
})
139+
})

0 commit comments

Comments
 (0)