From a8e3787afc47b792d1feb39e50ad3f765d41c4ac Mon Sep 17 00:00:00 2001 From: Brian Flad Date: Mon, 1 Apr 2019 20:10:32 -0400 Subject: [PATCH] helper/schema: Prevent setSet() panic with typed nil References: * https://github.com/hashicorp/terraform/issues/14418 * v0.9.5 (original bug report): https://github.com/hashicorp/terraform/blob/a59ee0b30e1350d18e44a3a2a405124302751f93/helper/schema/field_writer_map.go#L311 * v0.11.12 (Terraform AWS Provider discovery): https://github.com/hashicorp/terraform/blob/057286e5228559722bfc9bf6a03679650358c9d2/helper/schema/field_writer_map.go#L343 When creating flatten functions in Terraform Providers that return *schema.Set, its possible to return a typed `nil`, e.g. ```go func flattenHeaders(h *cloudfront.Headers) *schema.Set { if h.Items != nil { return schema.NewSet(schema.HashString, flattenStringList(h.Items)) } return nil } ``` This previously could cause a panic, e.g. ``` panic: runtime error: invalid memory address or nil pointer dereference [signal SIGSEGV: segmentation violation code=0x1 addr=0x8 pc=0x1881911] goroutine 1325 [running]: github.com/hashicorp/terraform/helper/schema.(*MapFieldWriter).setSet(0xc00054bf00, 0xc00073efa0, 0x5, 0x5, 0x5828140, 0x0, 0xc0002cea50, 0xc000e996a8, 0xc001026e40) /Users/bflad/go/pkg/mod/github.com/hashicorp/terraform@v0.11.12/helper/schema/field_writer_map.go:343 +0x211 ``` Here we catch the typed `nil` and return an empty list flatmap result instead. Unit testing result prior to code update: ``` --- FAIL: TestMapFieldWriter (0.00s) panic: runtime error: invalid memory address or nil pointer dereference [recovered] panic: runtime error: invalid memory address or nil pointer dereference [signal SIGSEGV: segmentation violation code=0x1 addr=0x8 pc=0x1777cdc] goroutine 913 [running]: testing.tRunner.func1(0xc00045b800) /usr/local/Cellar/go/1.12.1/libexec/src/testing/testing.go:830 +0x392 panic(0x192cf20, 0x2267ca0) /usr/local/Cellar/go/1.12.1/libexec/src/runtime/panic.go:522 +0x1b5 github.com/hashicorp/terraform/helper/schema.(*MapFieldWriter).setSet(0xc0004648a0, 0xc0004408d0, 0x1, 0x1, 0x19e3de0, 0x0, 0xc00045c600, 0x30, 0x19e0080) /Users/bflad/src/github.com/hashicorp/terraform/helper/schema/field_writer_map.go:344 +0x68c github.com/hashicorp/terraform/helper/schema.(*MapFieldWriter).set(0xc0004648a0, 0xc0004408d0, 0x1, 0x1, 0x19e3de0, 0x0, 0x1, 0x18) /Users/bflad/src/github.com/hashicorp/terraform/helper/schema/field_writer_map.go:107 +0x28b github.com/hashicorp/terraform/helper/schema.(*MapFieldWriter).WriteField(0xc0004648a0, 0xc0004408d0, 0x1, 0x1, 0x19e3de0, 0x0, 0x0, 0x0) /Users/bflad/src/github.com/hashicorp/terraform/helper/schema/field_writer_map.go:89 +0x504 github.com/hashicorp/terraform/helper/schema.TestMapFieldWriter(0xc00045b800) /Users/bflad/src/github.com/hashicorp/terraform/helper/schema/field_writer_map_test.go:337 +0x2ddd testing.tRunner(0xc00045b800, 0x1a44f90) /usr/local/Cellar/go/1.12.1/libexec/src/testing/testing.go:865 +0xc0 created by testing.(*T).Run /usr/local/Cellar/go/1.12.1/libexec/src/testing/testing.go:916 +0x35a ``` --- helper/schema/field_writer_map.go | 5 +++++ helper/schema/field_writer_map_test.go | 9 +++++++++ 2 files changed, 14 insertions(+) diff --git a/helper/schema/field_writer_map.go b/helper/schema/field_writer_map.go index 08ee11c37..c09358b1b 100644 --- a/helper/schema/field_writer_map.go +++ b/helper/schema/field_writer_map.go @@ -341,6 +341,11 @@ func (w *MapFieldWriter) setSet( // problems when the old data isn't wiped first. w.clearTree(addr) + if value.(*Set) == nil { + w.result[k+".#"] = "0" + return nil + } + for code, elem := range value.(*Set).m { if err := w.set(append(addrCopy, code), elem); err != nil { return err diff --git a/helper/schema/field_writer_map_test.go b/helper/schema/field_writer_map_test.go index 42ed9c208..d1a7932aa 100644 --- a/helper/schema/field_writer_map_test.go +++ b/helper/schema/field_writer_map_test.go @@ -203,6 +203,15 @@ func TestMapFieldWriter(t *testing.T) { }, }, + "set typed nil": { + []string{"set"}, + func() *Set { return nil }(), + false, + map[string]string{ + "set.#": "0", + }, + }, + "set resource": { []string{"setDeep"}, []interface{}{