Skip to content

Commit e46d245

Browse files
authored
Decode: don't crash on embedded nil pointers (#808)
Also has the perks of reducing the overhead of FindByIndex: ``` name old time/op new time/op delta UnmarshalDataset/config-32 17.0ms ± 1% 17.0ms ± 1% ~ (p=1.000 n=5+5) UnmarshalDataset/canada-32 71.6ms ± 1% 71.4ms ± 1% ~ (p=1.000 n=5+5) UnmarshalDataset/citm_catalog-32 24.2ms ± 3% 23.5ms ± 2% -3.03% (p=0.032 n=5+5) UnmarshalDataset/twitter-32 9.37ms ± 1% 9.09ms ± 2% -2.97% (p=0.032 n=5+5) UnmarshalDataset/code-32 75.4ms ± 2% 74.9ms ± 0% ~ (p=0.222 n=5+5) UnmarshalDataset/example-32 147µs ±10% 136µs ± 1% -7.14% (p=0.008 n=5+5) Unmarshal/SimpleDocument/struct-32 512ns ± 2% 500ns ± 0% -2.35% (p=0.008 n=5+5) Unmarshal/SimpleDocument/map-32 721ns ± 2% 702ns ± 1% -2.68% (p=0.008 n=5+5) Unmarshal/ReferenceFile/struct-32 40.1µs ± 0% 39.6µs ± 0% -1.30% (p=0.008 n=5+5) Unmarshal/ReferenceFile/map-32 62.3µs ± 1% 60.6µs ± 0% -2.83% (p=0.008 n=5+5) Unmarshal/HugoFrontMatter-32 10.8µs ± 1% 10.5µs ± 1% -2.86% (p=0.008 n=5+5) name old speed new speed delta UnmarshalDataset/config-32 61.8MB/s ± 1% 61.8MB/s ± 1% ~ (p=1.000 n=5+5) UnmarshalDataset/canada-32 30.8MB/s ± 1% 30.8MB/s ± 1% ~ (p=1.000 n=5+5) UnmarshalDataset/citm_catalog-32 23.0MB/s ± 3% 23.8MB/s ± 2% +3.09% (p=0.032 n=5+5) UnmarshalDataset/twitter-32 47.2MB/s ± 1% 48.6MB/s ± 2% +3.09% (p=0.032 n=5+5) UnmarshalDataset/code-32 35.6MB/s ± 2% 35.9MB/s ± 0% ~ (p=0.222 n=5+5) UnmarshalDataset/example-32 55.3MB/s ±10% 59.4MB/s ± 1% +7.36% (p=0.008 n=5+5) Unmarshal/SimpleDocument/struct-32 21.5MB/s ± 2% 22.0MB/s ± 0% +2.41% (p=0.008 n=5+5) Unmarshal/SimpleDocument/map-32 15.2MB/s ± 2% 15.7MB/s ± 1% +2.74% (p=0.008 n=5+5) Unmarshal/ReferenceFile/struct-32 131MB/s ± 0% 132MB/s ± 0% +1.31% (p=0.008 n=5+5) Unmarshal/ReferenceFile/map-32 84.1MB/s ± 1% 86.6MB/s ± 0% +2.91% (p=0.008 n=5+5) Unmarshal/HugoFrontMatter-32 50.6MB/s ± 1% 52.1MB/s ± 1% +2.93% (p=0.008 n=5+5) name old alloc/op new alloc/op delta UnmarshalDataset/config-32 5.86MB ± 0% 5.86MB ± 0% ~ (p=0.579 n=5+5) UnmarshalDataset/canada-32 83.0MB ± 0% 83.0MB ± 0% ~ (p=0.651 n=5+5) UnmarshalDataset/citm_catalog-32 34.7MB ± 0% 34.7MB ± 0% ~ (p=0.548 n=5+5) UnmarshalDataset/twitter-32 12.7MB ± 0% 12.7MB ± 0% ~ (p=1.000 n=5+5) UnmarshalDataset/code-32 22.2MB ± 0% 22.2MB ± 0% ~ (p=0.841 n=5+5) UnmarshalDataset/example-32 186kB ± 0% 186kB ± 0% ~ (p=0.111 n=5+5) Unmarshal/SimpleDocument/struct-32 805B ± 0% 805B ± 0% ~ (all equal) Unmarshal/SimpleDocument/map-32 1.13kB ± 0% 1.13kB ± 0% ~ (all equal) Unmarshal/ReferenceFile/struct-32 20.9kB ± 0% 20.9kB ± 0% ~ (p=0.643 n=5+5) Unmarshal/ReferenceFile/map-32 38.3kB ± 0% 38.3kB ± 0% ~ (p=0.397 n=5+5) Unmarshal/HugoFrontMatter-32 7.44kB ± 0% 7.44kB ± 0% ~ (all equal) name old allocs/op new allocs/op delta UnmarshalDataset/config-32 227k ± 0% 227k ± 0% ~ (p=1.000 n=5+5) UnmarshalDataset/canada-32 782k ± 0% 782k ± 0% ~ (all equal) UnmarshalDataset/citm_catalog-32 192k ± 0% 192k ± 0% ~ (p=0.968 n=4+5) UnmarshalDataset/twitter-32 56.9k ± 0% 56.9k ± 0% ~ (p=0.429 n=4+5) UnmarshalDataset/code-32 1.05M ± 0% 1.05M ± 0% ~ (p=0.556 n=4+5) UnmarshalDataset/example-32 1.36k ± 0% 1.36k ± 0% ~ (all equal) Unmarshal/SimpleDocument/struct-32 9.00 ± 0% 9.00 ± 0% ~ (all equal) Unmarshal/SimpleDocument/map-32 13.0 ± 0% 13.0 ± 0% ~ (all equal) Unmarshal/ReferenceFile/struct-32 183 ± 0% 183 ± 0% ~ (all equal) Unmarshal/ReferenceFile/map-32 642 ± 0% 642 ± 0% ~ (all equal) Unmarshal/HugoFrontMatter-32 141 ± 0% 141 ± 0% ~ (all equal) ``` Fixes #807
1 parent 7baa23f commit e46d245

File tree

2 files changed

+38
-4
lines changed

2 files changed

+38
-4
lines changed

unmarshaler.go

+23-4
Original file line numberDiff line numberDiff line change
@@ -123,7 +123,7 @@ type decoder struct {
123123
stashedExpr bool
124124

125125
// Skip expressions until a table is found. This is set to true when a
126-
// table could not be create (missing field in map), so all KV expressions
126+
// table could not be created (missing field in map), so all KV expressions
127127
// need to be skipped.
128128
skipUntilTable bool
129129

@@ -483,7 +483,7 @@ func (d *decoder) handleKeyPart(key ast.Iterator, v reflect.Value, nextFn handle
483483
d.errorContext.Struct = t
484484
d.errorContext.Field = path
485485

486-
f := v.FieldByIndex(path)
486+
f := fieldByIndex(v, path)
487487
x, err := nextFn(key, f)
488488
if err != nil || d.skipUntilTable {
489489
return reflect.Value{}, err
@@ -1071,7 +1071,7 @@ func (d *decoder) handleKeyValuePart(key ast.Iterator, value *ast.Node, v reflec
10711071
d.errorContext.Struct = t
10721072
d.errorContext.Field = path
10731073

1074-
f := v.FieldByIndex(path)
1074+
f := fieldByIndex(v, path)
10751075
x, err := d.handleKeyValueInner(key, value, f)
10761076
if err != nil {
10771077
return reflect.Value{}, err
@@ -1135,6 +1135,21 @@ func initAndDereferencePointer(v reflect.Value) reflect.Value {
11351135
return elem
11361136
}
11371137

1138+
// Same as reflect.Value.FieldByIndex, but creates pointers if needed.
1139+
func fieldByIndex(v reflect.Value, path []int) reflect.Value {
1140+
for i, x := range path {
1141+
v = v.Field(x)
1142+
1143+
if i < len(path)-1 && v.Kind() == reflect.Pointer {
1144+
if v.IsNil() {
1145+
v.Set(reflect.New(v.Type().Elem()))
1146+
}
1147+
v = v.Elem()
1148+
}
1149+
}
1150+
return v
1151+
}
1152+
11381153
type fieldPathsMap = map[string][]int
11391154

11401155
var globalFieldPathsCache atomic.Value // map[danger.TypeID]fieldPathsMap
@@ -1192,7 +1207,11 @@ func forEachField(t reflect.Type, path []int, do func(name string, path []int))
11921207
}
11931208

11941209
if f.Anonymous && name == "" {
1195-
forEachField(f.Type, fieldPath, do)
1210+
t2 := f.Type
1211+
if t2.Kind() == reflect.Pointer {
1212+
t2 = t2.Elem()
1213+
}
1214+
forEachField(t2, fieldPath, do)
11961215
continue
11971216
}
11981217

unmarshaler_test.go

+15
Original file line numberDiff line numberDiff line change
@@ -2451,6 +2451,21 @@ answer = 42
24512451
require.Error(t, err)
24522452
}
24532453

2454+
func TestIssue807(t *testing.T) {
2455+
type A struct {
2456+
Name string `toml:"name"`
2457+
}
2458+
2459+
type M struct {
2460+
*A
2461+
}
2462+
2463+
var m M
2464+
err := toml.Unmarshal([]byte(`name = 'foo'`), &m)
2465+
require.NoError(t, err)
2466+
require.Equal(t, "foo", m.Name)
2467+
}
2468+
24542469
func TestUnmarshalDecodeErrors(t *testing.T) {
24552470
examples := []struct {
24562471
desc string

0 commit comments

Comments
 (0)