From 7d76daf975dc0db59c2e4193e732a70513d4468a Mon Sep 17 00:00:00 2001 From: Dustin Long Date: Thu, 23 May 2024 10:12:29 -0400 Subject: [PATCH 1/2] Key lookup searches v.override using path prefixes When we assign to a map that uses urls for keys, we need to lookup values from that map in the same manner. Without doing so, `Get` will return inconsistent results. The agent codebase generally doesn't call `Get` this way, because we've known url keys work weird, but viper uses this type of call within AllSettings() in order to rebuild the config. In some cases, this could corrupt the data in the config. --- viper.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/viper.go b/viper.go index b77e682..6307c3c 100644 --- a/viper.go +++ b/viper.go @@ -1026,8 +1026,8 @@ func (v *Viper) find(lcaseKey string, skipDefault bool) interface{} { path = strings.Split(lcaseKey, v.keyDelim) nested = len(path) > 1 - // Set() override first - val = v.searchMap(v.override, path) + // Set() writes to override, so check override first + val = v.searchMapWithPathPrefixes(v.override, path) if val != nil { return val } From a53d9a5d7157c6428c1262e07bc1ff87b0399a2d Mon Sep 17 00:00:00 2001 From: Dustin Long Date: Thu, 23 May 2024 11:50:47 -0400 Subject: [PATCH 2/2] Add test case that demonstrates the fixed behavior --- viper_test.go | 47 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 47 insertions(+) diff --git a/viper_test.go b/viper_test.go index 5507a6d..cbf2ff0 100644 --- a/viper_test.go +++ b/viper_test.go @@ -49,6 +49,24 @@ eyes : brown beard: true `) +var yamlExampleWithURLKey = []byte(`Hacker: true +name: steve +hobbies: +- skateboarding +- snowboarding +- go +clothing: + jacket: leather + trousers: denim + pants: + size: large + http://example.com: + review +age: 35 +eyes : brown +beard: true +`) + var yamlExampleWithExtras = []byte(`Existing: true Bogus: true `) @@ -154,6 +172,10 @@ func initYAML(v *Viper) { initConfig(v, "yaml", string(yamlExample)) } +func initYAMLWithURLKey(v *Viper) { + initConfig(v, "yaml", string(yamlExampleWithURLKey)) +} + func initJSON(v *Viper) { v.SetConfigType("json") r := bytes.NewReader(jsonExample) @@ -1894,3 +1916,28 @@ func TestIsKnown(t *testing.T) { assert.False(t, v.IsKnown("unknown")) } + +func copyMap(m map[string]interface{}) map[string]interface{} { + res := make(map[string]interface{}) + for k, v := range m { + res[k] = v + } + return res +} + +func TestAssignToMapThenGet(t *testing.T) { + v := New() + initYAMLWithURLKey(v) + + // NOTE: copyMap is necessary here for the test to demonstrate the actual issue + // Without copying this map, this block of code will modify the v.defaults layer + // which hides the bug that v.Get should return the v.override value + clothingElem := copyMap(v.Get("clothing").(map[string]interface{})) + clothingElem["http://example.com"] = "recommendation" + v.Set("clothing", clothingElem) + + // demonstart that v.Get returns the overriden value, despite having a url in the + // key path, and the key not being set directly, but rather being set as a map key + expected := `recommendation` + require.Equal(t, expected, v.Get("clothing.http://example.com")) +}