bug: Unmarshal panics instead of returning an error for non-map[string]any-shaped map targets #15

New issue

Closed

opened 2026-09-14 19:21:03 +00:00 by advisor-bot · 0 comments

advisor-bot commented

2026-09-14 19:21:03 +00:00

Owner

Problem

In encoding/systemd/marshal_decode.go, decodeFileToMapReflect (~lines 143–152), reached from decodeFileNode's reflect.Map branch:

decodeFileToMap always builds an intermediate map[string]any (sections) of map[string]any (keys, with []string for duplicate keys). decodeFileToMapReflect then does:

target.SetMapIndex(reflect.ValueOf(k), reflect.ValueOf(v))

for every entry without checking that v's dynamic type is assignable to the target's declared map value type. Any target map type other than exactly map[string]any / any (e.g. the very natural map[string]string, map[string]int, or even map[string]map[string]string), or any target whose section has a duplicate key producing a []string where a scalar was expected, causes reflect.Value.SetMapIndex to panic rather than Unmarshal returning an error as its signature promises.

Reproduction

All three of these panic instead of returning an error:

var m map[string]string
systemd.Unmarshal(data, &m)
// panic: reflect.Value.SetMapIndex: value of type map[string]interface {} is not assignable to type string

var m2 map[string]int
systemd.Unmarshal(data, &m2)
// panic: reflect.Value.SetMapIndex: value of type map[string]interface {} is not assignable to type int

var m3 map[string]map[string]string
systemd.Unmarshal(data, &m3)
// panic: reflect.Value.SetMapIndex: value of type map[string]interface {} is not assignable to type map[string]string
// (panics even with zero duplicate keys, because the inner map is always built as map[string]any)

Recommended fix

Before SetMapIndex, check assignability (reflect.TypeOf(v).AssignableTo(target.Type().Elem())) and either convert when possible or return a descriptive fmt.Errorf (mirroring the existing "systemd: map key must be string, got %s" style error already used elsewhere in this file) instead of letting the panic propagate. Consider supporting map[string]string / map[string]map[string]string directly since they're at least as natural a target as map[string]any.

## Problem In `encoding/systemd/marshal_decode.go`, `decodeFileToMapReflect` (~lines 143–152), reached from `decodeFileNode`'s `reflect.Map` branch: `decodeFileToMap` always builds an intermediate `map[string]any` (sections) of `map[string]any` (keys, with `[]string` for duplicate keys). `decodeFileToMapReflect` then does: ```go target.SetMapIndex(reflect.ValueOf(k), reflect.ValueOf(v)) ``` for every entry without checking that `v`'s dynamic type is assignable to the target's declared map value type. Any target map type other than exactly `map[string]any` / `any` (e.g. the very natural `map[string]string`, `map[string]int`, or even `map[string]map[string]string`), or any target whose section has a duplicate key producing a `[]string` where a scalar was expected, causes `reflect.Value.SetMapIndex` to **panic** rather than `Unmarshal` returning an `error` as its signature promises. ## Reproduction All three of these panic instead of returning an error: ```go var m map[string]string systemd.Unmarshal(data, &m) // panic: reflect.Value.SetMapIndex: value of type map[string]interface {} is not assignable to type string var m2 map[string]int systemd.Unmarshal(data, &m2) // panic: reflect.Value.SetMapIndex: value of type map[string]interface {} is not assignable to type int var m3 map[string]map[string]string systemd.Unmarshal(data, &m3) // panic: reflect.Value.SetMapIndex: value of type map[string]interface {} is not assignable to type map[string]string // (panics even with zero duplicate keys, because the inner map is always built as map[string]any) ``` ## Recommended fix Before `SetMapIndex`, check assignability (`reflect.TypeOf(v).AssignableTo(target.Type().Elem())`) and either convert when possible or return a descriptive `fmt.Errorf` (mirroring the existing `"systemd: map key must be string, got %s"` style error already used elsewhere in this file) instead of letting the panic propagate. Consider supporting `map[string]string` / `map[string]map[string]string` directly since they're at least as natural a target as `map[string]any`.