From 5ee7d8630f8fe5f1834aab293ed42d03ea6b00c4 Mon Sep 17 00:00:00 2001 From: Seth Vargo Date: Fri, 5 Aug 2022 17:56:58 -0400 Subject: [PATCH] Track pointer initialization for zero values This fixes a bug where a pointer was initialized, but to the zero value. Like a *bool initialized to &false. Previously this would be considered "unset" because false is the zero value for a boolean. Now envconfig considers whether the pointer was initialized first, before checking whether the value is zero. --- envconfig.go | 6 +- envconfig_test.go | 166 ++++++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 163 insertions(+), 9 deletions(-) diff --git a/envconfig.go b/envconfig.go index 42e9d2f..3be3aad 100644 --- a/envconfig.go +++ b/envconfig.go @@ -304,6 +304,7 @@ func processWith(ctx context.Context, i interface{}, l Lookuper, parentNoInit bo } // Initialize pointer structs. + pointerWasSet := false for ef.Kind() == reflect.Ptr { if ef.IsNil() { if ef.Type().Elem().Kind() != reflect.Struct { @@ -316,6 +317,7 @@ func processWith(ctx context.Context, i interface{}, l Lookuper, parentNoInit bo // Use an empty struct of the type so we can traverse. ef = reflect.New(ef.Type().Elem()).Elem() } else { + pointerWasSet = true ef = ef.Elem() } } @@ -370,7 +372,7 @@ func processWith(ctx context.Context, i interface{}, l Lookuper, parentNoInit bo // The field already has a non-zero value and overwrite is false, do not // overwrite. - if !ef.IsZero() && !opts.Overwrite { + if (pointerWasSet || !ef.IsZero()) && !opts.Overwrite { continue } @@ -382,7 +384,7 @@ func processWith(ctx context.Context, i interface{}, l Lookuper, parentNoInit bo // If the field already has a non-zero value and there was no value directly // specified, do not overwrite the existing field. We only want to overwrite // when the envvar was provided directly. - if !ef.IsZero() && !found { + if (pointerWasSet || !ef.IsZero()) && !found { continue } diff --git a/envconfig_test.go b/envconfig_test.go index bbf97fd..32d5350 100644 --- a/envconfig_test.go +++ b/envconfig_test.go @@ -214,6 +214,18 @@ type Button struct { type Base64ByteSlice []Base64Bytes +func boolPtr(b bool) *bool { + return &b +} + +func stringPtr(s string) *string { + return &s +} + +func intPtr(i int) *int { + return &i +} + func TestProcessWith(t *testing.T) { t.Parallel() @@ -1445,35 +1457,49 @@ func TestProcessWith(t *testing.T) { // Pointer pointers { - name: "string_pointer", + name: "pointer_string", input: &struct { Field *string `env:"FIELD"` }{}, exp: &struct { Field *string `env:"FIELD"` }{ - Field: func() *string { s := "foo"; return &s }(), + Field: stringPtr("foo"), }, lookuper: MapLookuper(map[string]string{ "FIELD": "foo", }), }, { - name: "string_pointer_pointer", + name: "pointer_pointer_string", input: &struct { Field **string `env:"FIELD"` }{}, exp: &struct { Field **string `env:"FIELD"` }{ - Field: func() **string { s := "foo"; ptr := &s; return &ptr }(), + Field: func() **string { ptr := stringPtr("foo"); return &ptr }(), }, lookuper: MapLookuper(map[string]string{ "FIELD": "foo", }), }, { - name: "map_pointer", + name: "pointer_int", + input: &struct { + Field *int `env:"FIELD"` + }{}, + exp: &struct { + Field *int `env:"FIELD"` + }{ + Field: intPtr(5), + }, + lookuper: MapLookuper(map[string]string{ + "FIELD": "5", + }), + }, + { + name: "pointer_map", input: &struct { Field *map[string]string `env:"FIELD"` }{}, @@ -1490,7 +1516,7 @@ func TestProcessWith(t *testing.T) { }), }, { - name: "slice_pointer", + name: "pointer_slice", input: &struct { Field *[]string `env:"FIELD"` }{}, @@ -1507,7 +1533,21 @@ func TestProcessWith(t *testing.T) { }), }, { - name: "bool_pointer", + name: "pointer_bool", + input: &struct { + Field *bool `env:"FIELD"` + }{}, + exp: &struct { + Field *bool `env:"FIELD"` + }{ + Field: boolPtr(true), + }, + lookuper: MapLookuper(map[string]string{ + "FIELD": "true", + }), + }, + { + name: "pointer_bool_noinit", input: &struct { Field *bool `env:"FIELD,noinit"` }{}, @@ -1518,6 +1558,118 @@ func TestProcessWith(t *testing.T) { }, lookuper: MapLookuper(nil), }, + { + name: "pointer_bool_default_field_set_env_unset", + input: &struct { + Field *bool `env:"FIELD,default=true"` + }{ + Field: boolPtr(false), + }, + exp: &struct { + Field *bool `env:"FIELD,default=true"` + }{ + Field: boolPtr(false), + }, + lookuper: MapLookuper(nil), + }, + { + name: "pointer_bool_default_field_set_env_set", + input: &struct { + Field *bool `env:"FIELD,default=true"` + }{ + Field: boolPtr(false), + }, + exp: &struct { + Field *bool `env:"FIELD,default=true"` + }{ + Field: boolPtr(false), + }, + lookuper: MapLookuper(map[string]string{ + "FIELD": "true", + }), + }, + { + name: "pointer_bool_default_field_unset_env_set", + input: &struct { + Field *bool `env:"FIELD,default=true"` + }{}, + exp: &struct { + Field *bool `env:"FIELD,default=true"` + }{ + Field: boolPtr(false), + }, + lookuper: MapLookuper(map[string]string{ + "FIELD": "false", + }), + }, + { + name: "pointer_bool_default_field_unset_env_unset", + input: &struct { + Field *bool `env:"FIELD,default=true"` + }{}, + exp: &struct { + Field *bool `env:"FIELD,default=true"` + }{ + Field: boolPtr(true), + }, + lookuper: MapLookuper(nil), + }, + { + name: "pointer_bool_default_overwrite_field_set_env_unset", + input: &struct { + Field *bool `env:"FIELD,overwrite,default=true"` + }{ + Field: boolPtr(false), + }, + exp: &struct { + Field *bool `env:"FIELD,overwrite,default=true"` + }{ + Field: boolPtr(false), + }, + lookuper: MapLookuper(nil), + }, + { + name: "pointer_bool_default_overwrite_field_set_env_set", + input: &struct { + Field *bool `env:"FIELD,overwrite,default=true"` + }{ + Field: boolPtr(false), + }, + exp: &struct { + Field *bool `env:"FIELD,overwrite,default=true"` + }{ + Field: boolPtr(true), + }, + lookuper: MapLookuper(map[string]string{ + "FIELD": "true", + }), + }, + { + name: "pointer_bool_default_overwrite_field_unset_env_set", + input: &struct { + Field *bool `env:"FIELD,overwrite,default=true"` + }{}, + exp: &struct { + Field *bool `env:"FIELD,overwrite,default=true"` + }{ + Field: boolPtr(false), + }, + lookuper: MapLookuper(map[string]string{ + "FIELD": "false", + }), + }, + { + name: "pointer_bool_default_overwrite_field_unset_env_unset", + input: &struct { + Field *bool `env:"FIELD,overwrite,default=true"` + }{}, + exp: &struct { + Field *bool `env:"FIELD,overwrite,default=true"` + }{ + Field: boolPtr(true), + }, + lookuper: MapLookuper(nil), + }, // Marshallers {