From 9f860a7fd4f51a6deed5712699832aa1085e0c3e Mon Sep 17 00:00:00 2001 From: dualfroz Date: Fri, 4 Sep 2026 08:08:58 +0200 Subject: [PATCH] fix: MapKeys returns empty slice for natively-typed maps Get() preserves the concrete type of a map[string]T value loaded via Set() or a provider like structs, but MapKeys() only type-asserted map[string]any, so it silently reported no keys for anything else. Read keys via reflection instead so any map with string keys works. --- PR.md | 95 +++++++++++++++++++++++++++++++++++++++++++++ koanf.go | 15 ++++--- tests/koanf_test.go | 11 ++++++ 3 files changed, 116 insertions(+), 5 deletions(-) create mode 100644 PR.md diff --git a/PR.md b/PR.md new file mode 100644 index 0000000..d951d32 --- /dev/null +++ b/PR.md @@ -0,0 +1,95 @@ +# MapKeys returns an empty slice for a natively-typed map + +## Problem + +`Koanf.MapKeys(path)` is documented to return the sorted keys of the map at +`path`, or an empty slice "if the path is not a map". A value at a key path +does not have to be `map[string]any` to be a map, though: `Set()`, or a +provider such as `structs`, can put a `map[string]T` (`map[string]int64`, +`map[string]bool`, ...) directly into the config tree, and `Koanf.Get()` +returns it verbatim with its concrete type intact (`koanf.go`'s `Get` only +special-cases `map[string]any` for its copy, everything else falls through +to `copystructure.Copy`, which preserves the type). + +`MapKeys` only checked `o.(map[string]any)`, so a natively-typed map failed +that assertion and produced `[]string{}` even though the map plainly has +keys - a silent contract violation, not a "not a map" case. + +```go +k := koanf.New(".") +k.Load(confmap.Provider(map[string]any{ + "ints": map[string]int64{"a": 1, "b": 2}, +}, "."), nil) + +k.Get("ints") // map[string]int64{"a":1, "b":2} +k.MapKeys("ints") // []string{} -- should be []string{"a", "b"} +``` + +## Root cause + +`koanf.go:410-429`, `MapKeys`: + +```go +mp, ok := o.(map[string]any) +if !ok { + return out +} +``` + +This is the same shape of bug already fixed for `Int64Map`, `Float64Map` and +`BoolMap` in #444 (checking only `map[string]any` and silently dropping the +natively-typed sibling), applied to a different getter that PR does not +touch. + +## Fix + +Since `MapKeys` only needs the keys and does not care about the value type, +the fix reads them through reflection instead of enumerating every possible +`map[string]T`: any map whose key kind is `string` is accepted, `map[string]any` +included. + +```go +rv := reflect.ValueOf(o) +if rv.Kind() != reflect.Map || rv.Type().Key().Kind() != reflect.String { + return out +} + +out = make([]string, 0, rv.Len()) +for _, k := range rv.MapKeys() { + out = append(out, k.String()) +} +sort.Strings(out) +``` + +`reflect` and `sort` are already imported in `koanf.go` for other methods. + +## Verification + +`TestMapKeysNativeMap` in `tests/koanf_test.go` loads a `map[string]int64` +and asserts `MapKeys` returns its keys. + +Reverting only the `MapKeys` body in `koanf.go` (test left in place) fails it: + +``` +--- FAIL: TestMapKeysNativeMap + Error: Not equal: + expected: []string{"a", "b"} + actual : []string{} + Messages: map keys mismatch for a natively-typed map +``` + +`go build ./...`, `go vet ./...` and `gofmt -l .` are clean in both the root +module and `tests/` (the only `gofmt -l` hit is the pre-existing +`examples/read-huml/main.go`, untouched by this change). + +`go test ./... -count=1` and `go test ./... -count=1 -race` in `tests/`: +60 tests, all passing, including the new one. + +## Flags for the account owner + +- No CLA/DCO step found in `CONTRIBUTING.md` or `.github/`. +- Behaviour change: code that relied on `MapKeys` returning `[]string{}` for + a natively-typed map (rather than actually reading its keys) would see a + different result now. I think the old behaviour was simply wrong given the + doc comment, but flagging it since #444 called out the same kind of change + for its three getters. diff --git a/koanf.go b/koanf.go index fb57a50..63b2fb1 100644 --- a/koanf.go +++ b/koanf.go @@ -416,13 +416,18 @@ func (ko *Koanf) MapKeys(path string) []string { return out } - mp, ok := o.(map[string]any) - if !ok { + // Not every map at a key path is loaded as map[string]any. A value + // set directly with Set() or via a Provider such as structs keeps + // its native map[string]T type, so the keys have to be read through + // reflection to cover those too. + rv := reflect.ValueOf(o) + if rv.Kind() != reflect.Map || rv.Type().Key().Kind() != reflect.String { return out } - out = make([]string, 0, len(mp)) - for k := range mp { - out = append(out, k) + + out = make([]string, 0, rv.Len()) + for _, k := range rv.MapKeys() { + out = append(out, k.String()) } sort.Strings(out) return out diff --git a/tests/koanf_test.go b/tests/koanf_test.go index 39cbe92..86d35fe 100644 --- a/tests/koanf_test.go +++ b/tests/koanf_test.go @@ -1533,6 +1533,17 @@ func TestBoolsNativeSlice(t *testing.T) { assert.Equal([]bool{true, false, true}, k.Bools("bools")) } +func TestMapKeysNativeMap(t *testing.T) { + assert := assert.New(t) + + k := koanf.New(delim) + assert.NoError(k.Load(confmap.Provider(map[string]any{ + "ints": map[string]int64{"a": 1, "b": 2}, + }, "."), nil)) + + assert.Equal([]string{"a", "b"}, k.MapKeys("ints"), "map keys mismatch for a natively-typed map") +} + // waitTimeout waits for the waitgroup for the specified max timeout. // Returns true if waiting timed out. func waitTimeout(wg *sync.WaitGroup, timeout time.Duration) bool {