From e46d245c090232c37469a70ca74b6dd8dbc47f16 Mon Sep 17 00:00:00 2001 From: Thomas Pelletier Date: Sat, 20 Aug 2022 21:24:03 -0400 Subject: [PATCH] Decode: don't crash on embedded nil pointers (#808) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- unmarshaler.go | 27 +++++++++++++++++++++++---- unmarshaler_test.go | 15 +++++++++++++++ 2 files changed, 38 insertions(+), 4 deletions(-) diff --git a/unmarshaler.go b/unmarshaler.go index 6f196e4..2c526e9 100644 --- a/unmarshaler.go +++ b/unmarshaler.go @@ -123,7 +123,7 @@ type decoder struct { stashedExpr bool // Skip expressions until a table is found. This is set to true when a - // table could not be create (missing field in map), so all KV expressions + // table could not be created (missing field in map), so all KV expressions // need to be skipped. skipUntilTable bool @@ -483,7 +483,7 @@ func (d *decoder) handleKeyPart(key ast.Iterator, v reflect.Value, nextFn handle d.errorContext.Struct = t d.errorContext.Field = path - f := v.FieldByIndex(path) + f := fieldByIndex(v, path) x, err := nextFn(key, f) if err != nil || d.skipUntilTable { return reflect.Value{}, err @@ -1071,7 +1071,7 @@ func (d *decoder) handleKeyValuePart(key ast.Iterator, value *ast.Node, v reflec d.errorContext.Struct = t d.errorContext.Field = path - f := v.FieldByIndex(path) + f := fieldByIndex(v, path) x, err := d.handleKeyValueInner(key, value, f) if err != nil { return reflect.Value{}, err @@ -1135,6 +1135,21 @@ func initAndDereferencePointer(v reflect.Value) reflect.Value { return elem } +// Same as reflect.Value.FieldByIndex, but creates pointers if needed. +func fieldByIndex(v reflect.Value, path []int) reflect.Value { + for i, x := range path { + v = v.Field(x) + + if i < len(path)-1 && v.Kind() == reflect.Pointer { + if v.IsNil() { + v.Set(reflect.New(v.Type().Elem())) + } + v = v.Elem() + } + } + return v +} + type fieldPathsMap = map[string][]int 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)) } if f.Anonymous && name == "" { - forEachField(f.Type, fieldPath, do) + t2 := f.Type + if t2.Kind() == reflect.Pointer { + t2 = t2.Elem() + } + forEachField(t2, fieldPath, do) continue } diff --git a/unmarshaler_test.go b/unmarshaler_test.go index def6434..43a334e 100644 --- a/unmarshaler_test.go +++ b/unmarshaler_test.go @@ -2451,6 +2451,21 @@ answer = 42 require.Error(t, err) } +func TestIssue807(t *testing.T) { + type A struct { + Name string `toml:"name"` + } + + type M struct { + *A + } + + var m M + err := toml.Unmarshal([]byte(`name = 'foo'`), &m) + require.NoError(t, err) + require.Equal(t, "foo", m.Name) +} + func TestUnmarshalDecodeErrors(t *testing.T) { examples := []struct { desc string