From f7e427ae3fc06b8338470e276556c27b17c683b9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Petr=20Balv=C3=ADn?= Date: Tue, 22 Sep 2026 21:15:00 +0200 Subject: [PATCH] fix(decode): allocate embedded pointer maps and settle case collisions Assisted-by: GLM 5.3 --- decode.go | 25 ++++++++++++--- decode_test.go | 82 ++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 103 insertions(+), 4 deletions(-) diff --git a/decode.go b/decode.go index e790e39..118101a 100644 --- a/decode.go +++ b/decode.go @@ -7,6 +7,7 @@ import ( "context" "encoding" "fmt" + "maps" "reflect" "slices" "strings" @@ -337,7 +338,8 @@ func (d *decoder) assignStruct(tbl map[string]any, dst reflect.Value) error { if len(schema.required) > 0 { seen = make(map[string]bool, len(tbl)) } - for key, val := range tbl { + for _, key := range d.tableKeys(tbl) { + val := tbl[key] // A key that is already lowercase, which document keys usually are, // hits the map directly; only a miss pays for the case fold. resolved := key @@ -349,12 +351,15 @@ func (d *decoder) assignStruct(tbl map[string]any, dst reflect.Value) error { if !ok { if schema.embedMaps != nil { // Leftover keys land in an untagged embedded map, the inverse - // of the encoder inlining that map's entries. + // of the encoder inlining that map's entries. The assign call + // rather than assignMap itself lets it allocate the embedded + // pointer the field may be, the way any other destination is + // reached. mv, err := fieldByIndex(dst, schema.embedMaps[0]) if err != nil { return newDecodeError(key, err) } - if err := d.assignMap(map[string]any{key: val}, mv); err != nil { + if err := d.assign(map[string]any{key: val}, mv); err != nil { return newDecodeError(key, err) } } @@ -379,6 +384,18 @@ func (d *decoder) assignStruct(tbl map[string]any, dst reflect.Value) error { return nil } +// tableKeys returns the keys of tbl in the order the document wrote them +// when the node index knows it, and in sorted order otherwise, the order a +// hand-built tree or a node-free parse offers. The order settles which of +// two keys that differ only in case wins one field: the same key wins every +// run, instead of whichever a map iteration happened to hand out. +func (d *decoder) tableKeys(tbl map[string]any) []string { + if node := d.nodeOf(tbl); node != nil { + return node.Keys() + } + return slices.Sorted(maps.Keys(tbl)) +} + func (d *decoder) assignMap(tbl map[string]any, dst reflect.Value) error { if dst.Type().Key().Kind() != reflect.String { return fmt.Errorf("interpres: map key must be a string, got %s", dst.Type().Key()) @@ -499,7 +516,7 @@ func (d *decoder) setLocalTimeValue(t time.Time, dst reflect.Value) error { t.Hour(), t.Minute(), t.Second(), t.Nanosecond(), d.loc))) return nil } - return fmt.Errorf("interpres: cannot assign local date-time to time.Time; set Decoder.LocalTimeLocation to choose the zone") + return fmt.Errorf("interpres: cannot assign local date-time to time.Time; set LocalTimeLocation to choose the zone") } return fmt.Errorf("interpres: cannot assign local date-time to %s", dst.Type()) } diff --git a/decode_test.go b/decode_test.go index 6000799..8b5a251 100644 --- a/decode_test.go +++ b/decode_test.go @@ -1690,3 +1690,85 @@ func TestErrorMessagesGolden(t *testing.T) { }) } } + +// TestLocalTimeLocationLeavesOffsetsAlone pins that the option's zone is +// used for local date-times only: an offset date-time keeps the offset the +// document wrote. +func TestLocalTimeLocationLeavesOffsetsAlone(t *testing.T) { + var cfg struct { + Stamp time.Time `toml:"stamp"` + } + err := Unmarshal([]byte("stamp = 1979-05-27T07:32:00-07:00\n"), &cfg, + LocalTimeLocation(time.FixedZone("Prague", 2*60*60))) + if err != nil { + t.Fatalf("unmarshal: %v", err) + } + if _, off := cfg.Stamp.Zone(); off != -7*60*60 { + t.Errorf("offset = %d, want the document's -07:00", off/3600) + } +} + +// TestUnmarshalCaseCollisionIsDeterministic pins that two keys differing +// only in case, both matching one field, resolve the same way on every run +// and on both decode paths. +func TestUnmarshalCaseCollisionIsDeterministic(t *testing.T) { + type cfg struct { + Host string `toml:"host"` + } + in := []byte("Host = \"upper\"\nhost = \"lower\"\n") + // The tree path iterates a map, so pin the winner across many runs. + var want string + for range 50 { + var viaTree cfg + if err := treeDecodeInto(in, &viaTree); err != nil { + t.Fatalf("tree decode: %v", err) + } + if want == "" { + want = viaTree.Host + } else if viaTree.Host != want { + t.Fatalf("tree decode is not deterministic: %q then %q", want, viaTree.Host) + } + } + var targeted cfg + if err := Unmarshal(in, &targeted); err != nil { + t.Fatalf("unmarshal: %v", err) + } + if targeted.Host != want { + t.Errorf("targeted Host = %q, tree %q", targeted.Host, want) + } +} + +// TestUnmarshalEmbeddedPointerMap pins that leftover keys reach an embedded +// pointer to a map, allocating it, rather than panicking on the pointer. +func TestUnmarshalEmbeddedPointerMap(t *testing.T) { + type Extra map[string]int + type cfg struct { + *Extra + Name string `toml:"name"` + } + var c cfg + err := Unmarshal([]byte("name = \"x\"\nrogue = 7\n"), &c) + if err != nil { + t.Fatalf("unmarshal: %v", err) + } + if c.Extra == nil || (*c.Extra)["rogue"] != 7 { + t.Errorf("embedded map = %v, want rogue allocated and filled", c.Extra) + } +} + +// TestUnmarshalIgnoresEncodeTagOptions pins that the emission-only tag +// options change nothing on the decode side. +func TestUnmarshalIgnoresEncodeTagOptions(t *testing.T) { + type cfg struct { + Name string `toml:"name,omitempty"` + Port int `toml:"port,omitzero,comment=The port"` + } + var c cfg + err := Unmarshal([]byte("name = \"x\"\nport = 8080\n"), &c) + if err != nil { + t.Fatalf("unmarshal: %v", err) + } + if c.Name != "x" || c.Port != 8080 { + t.Errorf("cfg = %+v, want both fields filled", c) + } +}