Skip to content

Commit

Permalink
Track pointer initialization for zero values
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@sethvargo
sethvargo committed Aug 5, 2022
1 parent 63e4089 commit 5ee7d86
Show file tree
Hide file tree
Showing 2 changed files with 163 additions and 9 deletions.
6 changes: 4 additions & 2 deletions envconfig.go
View file
Expand Up @@ -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 {
Expand All @@ -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()
}
}
Expand Down Expand Up @@ -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
}

Expand All @@ -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
}

Expand Down
166 changes: 159 additions & 7 deletions envconfig_test.go
View file
Expand Up @@ -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()

Expand Down Expand Up @@ -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"`
}{},
Expand All @@ -1490,7 +1516,7 @@ func TestProcessWith(t *testing.T) {
}),
},
{
name: "slice_pointer",
name: "pointer_slice",
input: &struct {
Field *[]string `env:"FIELD"`
}{},
Expand All @@ -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"`
}{},
Expand All @@ -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
{
Expand Down

0 comments on commit 5ee7d86

Please sign in to comment.