Skip to content

Commit f329e88

Browse files
authored
fix: Options.Environment: Do not merge with default env (#353)
The commit 6f3a5c0 from PR #349, merged between the last two releases, changed the parsing logic for Options, especially allowing to merge map types. While this was already the case for Options.FuncMap, this breaks the API promise of Options.Environment to set "[e]nvironment keys and values that will be accessible for the service". In particular, this allowed an environment bleed while explicitly setting a custom environment, e.g., for testing purposes. This change reverts merging a custom Options.Environment with the default environment, if set.
1 parent 4ebfdad commit f329e88

File tree

2 files changed

+37
-4
lines changed

2 files changed

+37
-4
lines changed

env.go

+5-4
Original file line numberDiff line numberDiff line change
@@ -195,19 +195,20 @@ func mergeOptions[T any](target, source *T) {
195195

196196
targetType := targetPtr.Type()
197197
for i := 0; i < targetPtr.NumField(); i++ {
198+
fieldName := targetType.Field(i).Name
198199
targetField := targetPtr.Field(i)
199-
sourceField := sourcePtr.FieldByName(targetType.Field(i).Name)
200+
sourceField := sourcePtr.FieldByName(fieldName)
200201

201202
if targetField.CanSet() && !isZero(sourceField) {
202-
switch targetField.Kind() {
203-
case reflect.Map:
203+
// FuncMaps are being merged, while Environments must be overwritten
204+
if fieldName == "FuncMap" {
204205
if !sourceField.IsZero() {
205206
iter := sourceField.MapRange()
206207
for iter.Next() {
207208
targetField.SetMapIndex(iter.Key(), iter.Value())
208209
}
209210
}
210-
default:
211+
} else {
211212
targetField.Set(sourceField)
212213
}
213214
}

env_test.go

+32
Original file line numberDiff line numberDiff line change
@@ -2322,3 +2322,35 @@ func TestIssue350(t *testing.T) {
23222322
isNoErr(t, Parse(&cfg))
23232323
isEqual(t, map[string]string{"url": "https://foo.bar:2030"}, cfg.Map)
23242324
}
2325+
2326+
func TestEnvBleed(t *testing.T) {
2327+
type Config struct {
2328+
Foo string `env:"FOO"`
2329+
}
2330+
2331+
t.Setenv("FOO", "101")
2332+
2333+
t.Run("Default env with value", func(t *testing.T) {
2334+
var cfg Config
2335+
isNoErr(t, ParseWithOptions(&cfg, Options{}))
2336+
isEqual(t, "101", cfg.Foo)
2337+
})
2338+
2339+
t.Run("Empty env without value", func(t *testing.T) {
2340+
var cfg Config
2341+
isNoErr(t, ParseWithOptions(&cfg, Options{Environment: map[string]string{}}))
2342+
isEqual(t, "", cfg.Foo)
2343+
})
2344+
2345+
t.Run("Custom env with overwritten value", func(t *testing.T) {
2346+
var cfg Config
2347+
isNoErr(t, ParseWithOptions(&cfg, Options{Environment: map[string]string{"FOO": "202"}}))
2348+
isEqual(t, "202", cfg.Foo)
2349+
})
2350+
2351+
t.Run("Custom env without value", func(t *testing.T) {
2352+
var cfg Config
2353+
isNoErr(t, ParseWithOptions(&cfg, Options{Environment: map[string]string{"BAR": "202"}}))
2354+
isEqual(t, "", cfg.Foo)
2355+
})
2356+
}

0 commit comments

Comments
 (0)