diff --git a/env.go b/env.go index 93d7a76c..ca0a8f11 100644 --- a/env.go +++ b/env.go @@ -168,6 +168,12 @@ type Options struct { // Useful for mixing default values from `envDefault` and struct initialization SetDefaultsForZeroValuesOnly bool + // AllowEmpty, when true, treats an explicitly-set-but-empty env variable as + // a real empty value instead of falling back to envDefault. The default + // behaviour (false) keeps the historical "empty env means use default" + // shortcut for backwards compatibility. + AllowEmpty bool + // Custom parse functions for different types. FuncMap map[reflect.Type]ParserFunc @@ -499,12 +505,17 @@ func doParseSlice(ref reflect.Value, processField processFieldFn, opts Options) } func setField(refField reflect.Value, refTypeField reflect.StructField, opts Options, fieldParams FieldParams) error { - value, err := get(fieldParams, opts) + value, isDefault, err := get(fieldParams, opts) if err != nil { return err } - if value != "" && (!opts.SetDefaultsForZeroValuesOnly || refField.IsZero()) { + // SetDefaultsForZeroValuesOnly is about defaults: a real env value + // should still win over a non-zero field. The previous form treated + // any non-empty value as a default, so an explicit env override was + // silently dropped when the destination already held something + // non-zero. See #364. + if value != "" && (!isDefault || !opts.SetDefaultsForZeroValuesOnly || refField.IsZero()) { return set(refField, refTypeField, value, opts.FuncMap) } @@ -590,14 +601,15 @@ func parseFieldParams(field reflect.StructField, opts Options) (FieldParams, err return result, nil } -func get(fieldParams FieldParams, opts Options) (val string, err error) { - var exists, isDefault bool +func get(fieldParams FieldParams, opts Options) (val string, isDefault bool, err error) { + var exists bool val, exists, isDefault = getOr( fieldParams.Key, fieldParams.DefaultValue, fieldParams.HasDefaultValue, opts.Environment, + opts.AllowEmpty, ) if fieldParams.Expand { @@ -611,18 +623,18 @@ func get(fieldParams FieldParams, opts Options) (val string, err error) { } if fieldParams.Required && !exists && fieldParams.OwnKey != "" { - return "", newVarIsNotSetError(fieldParams.Key) + return "", false, newVarIsNotSetError(fieldParams.Key) } if fieldParams.NotEmpty && val == "" { - return "", newEmptyVarError(fieldParams.Key) + return "", false, newEmptyVarError(fieldParams.Key) } if fieldParams.LoadFile && val != "" { filename := val val, err = getFromFile(filename) if err != nil { - return "", newLoadFileContentError(filename, fieldParams.Key, err) + return "", false, newLoadFileContentError(filename, fieldParams.Key, err) } } @@ -631,7 +643,7 @@ func get(fieldParams FieldParams, opts Options) (val string, err error) { opts.OnSet(fieldParams.Key, val, isDefault) } } - return val, err + return val, isDefault, err } // split the env tag's key into the expected key and desired option, if any. @@ -645,12 +657,12 @@ func getFromFile(filename string) (value string, err error) { return string(b), err } -func getOr(key, defaultValue string, defExists bool, envs map[string]string) (val string, exists, isDefault bool) { +func getOr(key, defaultValue string, defExists bool, envs map[string]string, allowEmpty bool) (val string, exists, isDefault bool) { value, exists := envs[key] switch { case (!exists || key == "") && defExists: return defaultValue, true, true - case exists && value == "" && defExists: + case exists && value == "" && defExists && !allowEmpty: return defaultValue, true, true case !exists: return "", false, false diff --git a/env_test.go b/env_test.go index 2fe79c31..c5496690 100644 --- a/env_test.go +++ b/env_test.go @@ -2259,6 +2259,57 @@ func TestSetDefaultsForZeroValuesOnly(t *testing.T) { } } +// Regression for #406. By default an explicitly-set-but-empty env value +// falls back to envDefault. AllowEmpty:true keeps the empty value so +// callers can clear out a string via the environment. +func TestAllowEmpty(t *testing.T) { + type config struct { + Str string `env:"STR" envDefault:"foo"` + } + + t.Run("default keeps falling back to envDefault", func(t *testing.T) { + var cfg config + isNoErr(t, ParseWithOptions(&cfg, Options{ + Environment: map[string]string{"STR": ""}, + })) + isEqual(t, "foo", cfg.Str) + }) + + t.Run("AllowEmpty passes the empty value through", func(t *testing.T) { + var cfg config + isNoErr(t, ParseWithOptions(&cfg, Options{ + Environment: map[string]string{"STR": ""}, + AllowEmpty: true, + })) + isEqual(t, "", cfg.Str) + }) + + t.Run("AllowEmpty still uses default when env is unset", func(t *testing.T) { + var cfg config + isNoErr(t, ParseWithOptions(&cfg, Options{ + Environment: map[string]string{}, + AllowEmpty: true, + })) + isEqual(t, "foo", cfg.Str) + }) +} + +// Regression for #364: SetDefaultsForZeroValuesOnly is about defaults. +// An env value set in the environment should still win over a non-zero +// field, otherwise users can't override pre-populated config via env at +// all. +func TestSetDefaultsForZeroValuesOnly_EnvOverridesNonZero(t *testing.T) { + type Config struct { + Username string `env:"USERNAME" envDefault:"admin"` + } + cfg := Config{Username: "root"} + isNoErr(t, ParseWithOptions(&cfg, Options{ + Environment: map[string]string{"USERNAME": "user1"}, + SetDefaultsForZeroValuesOnly: true, + })) + isEqual(t, "user1", cfg.Username) +} + func TestParseWithOptionsRenamedPrefix(t *testing.T) { type Config struct { Str string `env:"STR"`