Skip to content

Commit 74ddef6

Browse files
Disallow empty root struct
Resolves #341 Emptry root struct cannot work since decoding it is a noop and never reaches the end of column, thus potentially allowing a very large number of such empty records to be "decoded" by Reader without making any progress. This makes protecting from invalid inputs hard. We now disallow this, since there is no plausible use case for a schema like this. We still allow empty non-root structs, which is a valid way to have a schema that can evolve by adding fields to empty struct. It does not suffer from the same problem since Reader will have to make some progress for non-empty root struct and will eventually reach end of column for root struct. We also disallow this in stefc schema parser.
1 parent c60f5b9 commit 74ddef6

11 files changed

Lines changed: 57 additions & 11 deletions

File tree

examples/ints/internal/ints/record.go

Lines changed: 4 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

examples/jsonl/internal/jsonstef/record.go

Lines changed: 4 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

examples/profile/internal/profile/sample.go

Lines changed: 4 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

go/otel/otelstef/metrics.go

Lines changed: 4 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

go/otel/otelstef/spans.go

Lines changed: 4 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
go test fuzz v1
2+
[]byte("STEF\x0200\x00 \n\x01\x0000000000\x02\x040000\x00\x000000000000000\x010\xca\xd6\xcd\xc2\xf9\x7f\tA0\x03\xe800000")

go/pkg/errors.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ var ErrTotalColumnSizeLimitExceeded = NewDecodeError("total column size limit ex
2020
var ErrRecordAllocLimitExceeded = NewDecodeError("record allocation limit exceeded")
2121

2222
var ErrTooManyFieldsToDecode = NewDecodeError("too many fields to decode")
23+
var ErrEmptyRootStructDisallowed = NewDecodeError("cannot decode empty root struct")
2324

2425
type DecodeError struct {
2526
msg string

go/pkg/idl/parser.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -192,6 +192,10 @@ func (p *Parser) parseStruct(isOneOf bool) (*schema.Struct, error) {
192192
return nil, err
193193
}
194194

195+
if str.IsRoot && len(str.Fields) == 0 {
196+
return nil, p.error("root struct must have at least one field")
197+
}
198+
195199
if err := p.eat(tRBrace); err != nil {
196200
return nil, err
197201
}

go/pkg/idl/parser_test.go

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,10 @@ func TestParserErrors(t *testing.T) {
8282
input: "package abc oneof Oneof dict(abc) {}",
8383
err: "test.stef:1:25: oneof cannot have dict modifier",
8484
},
85+
{
86+
input: "package abc struct Empty root {}",
87+
err: "test.stef:1:33: root struct must have at least one field",
88+
},
8589
}
8690

8791
for _, test := range tests {
@@ -99,30 +103,30 @@ func TestParserWarnings(t *testing.T) {
99103
warn []string
100104
}{
101105
{
102-
input: "package abc struct R root {} struct U {}",
106+
input: "package abc struct R root { x bool } struct U {}",
103107
warn: []string{`Warning: struct "U" is defined but not used`},
104108
},
105109
{
106-
input: "package abc struct R root {} multimap M { key string value int64 }",
110+
input: "package abc struct R root { x bool } multimap M { key string value int64 }",
107111
warn: []string{`Warning: multimap "M" is defined but not used`},
108112
},
109113
{
110-
input: "package abc struct R root {} oneof O {}",
114+
input: "package abc struct R root { x bool } oneof O {}",
111115
warn: []string{`Warning: oneof "O" is defined but not used`},
112116
},
113117
{
114-
input: "package abc struct R root {} enum E {}",
118+
input: "package abc struct R root { x bool } enum E {}",
115119
warn: []string{`Warning: enum "E" is defined but not used`},
116120
},
117121
{
118-
input: "package abc struct R root {} struct A { B B optional } struct B { A A optional }",
122+
input: "package abc struct R root { x bool } struct A { B B optional } struct B { A A optional }",
119123
warn: []string{
120124
`Warning: struct "A" is defined but not used`, `Warning: struct "B" is defined but not used`,
121125
},
122126
},
123127
// Multiple unused types of different kinds
124128
{
125-
input: "package abc struct R root {} struct U1 {} struct U2 {} oneof O {} multimap M { key string value int64 } enum E {}",
129+
input: "package abc struct R root { x bool } struct U1 {} struct U2 {} oneof O {} multimap M { key string value int64 } enum E {}",
126130
warn: []string{
127131
`Warning: oneof "O" is defined but not used`,
128132
`Warning: struct "U1" is defined but not used`,
@@ -133,7 +137,7 @@ func TestParserWarnings(t *testing.T) {
133137
},
134138
// Chain of unused structs where one references another
135139
{
136-
input: "package abc struct R root {} struct A { b B } struct B { c C } struct C {}",
140+
input: "package abc struct R root { x bool } struct A { b B } struct B { c C } struct C {}",
137141
warn: []string{
138142
`Warning: struct "A" is defined but not used`,
139143
`Warning: struct "B" is defined but not used`,
@@ -142,7 +146,7 @@ func TestParserWarnings(t *testing.T) {
142146
},
143147
// Circular dependency that's unused
144148
{
145-
input: "package abc struct R root {} struct A { b []B c C } struct B { a A } struct C { name string }",
149+
input: "package abc struct R root { x bool } struct A { b []B c C } struct B { a A } struct C { name string }",
146150
warn: []string{
147151
`Warning: struct "A" is defined but not used`,
148152
`Warning: struct "B" is defined but not used`,
@@ -205,14 +209,14 @@ func TestParserWarnings(t *testing.T) {
205209
},
206210
// Self-referencing unused struct
207211
{
208-
input: "package abc struct R root {} struct SelfRef { next SelfRef optional }",
212+
input: "package abc struct R root { x bool } struct SelfRef { next SelfRef optional }",
209213
warn: []string{
210214
`Warning: struct "SelfRef" is defined but not used`,
211215
},
212216
},
213217
// Multiple levels of mutual references that are unused
214218
{
215-
input: "package abc struct R root {} struct A { b B } struct B { c C } struct C { a A optional }",
219+
input: "package abc struct R root { x bool } struct A { b B } struct B { c C } struct C { a A optional }",
216220
warn: []string{
217221
`Warning: struct "A" is defined but not used`,
218222
`Warning: struct "B" is defined but not used`,

go/pkg/schema/utils.go

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,12 @@ func ShrinkRandomly(r *rand.Rand, schem *Schema) bool {
1515
var structNames []string
1616
for structName := range schem.Structs {
1717
structNames = append(structNames, structName)
18-
totalFieldCount += len(schem.Structs[structName].Fields)
18+
str := schem.Structs[structName]
19+
shrinkableCount := len(str.Fields)
20+
if str.IsRoot {
21+
shrinkableCount-- // Do not remove the last field from the root struct
22+
}
23+
totalFieldCount += shrinkableCount
1924
}
2025
if totalFieldCount == 0 {
2126
// Nothing to shrink
@@ -34,6 +39,11 @@ func ShrinkRandomly(r *rand.Rand, schem *Schema) bool {
3439
}
3540

3641
func shrinkStruct(r *rand.Rand, schem *Schema, str *Struct) bool {
42+
if str.IsRoot && len(str.Fields) <= 1 {
43+
// Do not remove the last field from the root struct
44+
return false
45+
}
46+
3747
if r.IntN(10) == 0 && len(str.Fields) > 0 {
3848
str.Fields = str.Fields[0 : len(str.Fields)-1]
3949
return true

0 commit comments

Comments
 (0)