From 79d18993408409c6dec0a6b4351e3bf1b0b96a58 Mon Sep 17 00:00:00 2001 From: Reuven Harrison Date: Sun, 2 Aug 2026 22:52:39 +0300 Subject: [PATCH] Add Decoder.AllowDuplicateKeys A repeated mapping key is an error here, which is YAML's rule, while encoding/json takes the last occurrence (RFC 8259 says object names SHOULD be unique, so the handling is the implementation's to choose). That difference is invisible until one program decodes both formats into the same types, at which point a document loads or fails depending only on which format it happens to be written in. This exposes the existing internal switch so such a caller can pick one answer for both. The default is unchanged: repeated keys still error. Enabling the option makes the last occurrence win, matching encoding/json. Tests assert the surviving value rather than just the absence of an error, since "does not fail" would equally be satisfied by dropping the key or keeping the first, and neither matches encoding/json. They also cover the struct and generic-map decode paths separately (different code in the decoder), nesting, and that the flag does not leak between decoders. --- duplicate_keys_test.go | 90 ++++++++++++++++++++++++++++++++++++++++++ yaml.go | 27 ++++++++++--- 2 files changed, 112 insertions(+), 5 deletions(-) create mode 100644 duplicate_keys_test.go diff --git a/duplicate_keys_test.go b/duplicate_keys_test.go new file mode 100644 index 00000000..b1d2ac61 --- /dev/null +++ b/duplicate_keys_test.go @@ -0,0 +1,90 @@ +package yaml_test + +import ( + "bytes" + + yaml "github.com/oasdiff/yaml3" + . "gopkg.in/check.v1" +) + +type dupInfo struct { + Title string `yaml:"title"` + Version string `yaml:"version"` +} + +type dupDoc struct { + Info dupInfo `yaml:"info"` +} + +const dupSrc = "info:\n title: a\n title: b\n version: \"1\"\n" + +func decodeDup(c *C, allow bool) (dupDoc, error) { + dec := yaml.NewDecoder(bytes.NewBufferString(dupSrc)) + dec.AllowDuplicateKeys(allow) + var d dupDoc + return d, dec.Decode(&d) +} + +// The default is unchanged: a repeated key is an error, which is YAML's rule. +func (s *S) TestDuplicateKeys_RejectedByDefault(c *C) { + dec := yaml.NewDecoder(bytes.NewBufferString(dupSrc)) + var d dupDoc + err := dec.Decode(&d) + c.Assert(err, NotNil) + c.Assert(err.Error(), Matches, `(?s).*already defined.*`) +} + +func (s *S) TestDuplicateKeys_RejectedWhenDisallowed(c *C) { + _, err := decodeDup(c, false) + c.Assert(err, NotNil) +} + +// Enabled, the last occurrence wins -- the same answer encoding/json gives for +// a repeated JSON object name. Asserting the surviving value, not merely the +// absence of an error: "does not fail" would also be satisfied by dropping the +// key or keeping the first, and neither would match encoding/json. +func (s *S) TestDuplicateKeys_LastOneWinsWhenAllowed(c *C) { + d, err := decodeDup(c, true) + c.Assert(err, IsNil) + c.Assert(d.Info.Title, Equals, "b") + // Sibling keys are untouched by the override. + c.Assert(d.Info.Version, Equals, "1") +} + +// The flag is per-decoder, so enabling it for one document must not leak into +// another built from the same package. +func (s *S) TestDuplicateKeys_FlagDoesNotLeak(c *C) { + _, err := decodeDup(c, true) + c.Assert(err, IsNil) + + dec := yaml.NewDecoder(bytes.NewBufferString(dupSrc)) + var d dupDoc + c.Assert(dec.Decode(&d), NotNil) +} + +// A duplicate inside a nested mapping is caught too, not just at the top level. +func (s *S) TestDuplicateKeys_Nested(c *C) { + const src = "a:\n b:\n c: 1\n c: 2\n" + var out any + + dec := yaml.NewDecoder(bytes.NewBufferString(src)) + c.Assert(dec.Decode(&out), NotNil) + + dec = yaml.NewDecoder(bytes.NewBufferString(src)) + dec.AllowDuplicateKeys(true) + out = nil + c.Assert(dec.Decode(&out), IsNil) + a := out.(map[string]any)["a"].(map[string]any) + c.Assert(a["b"].(map[string]any)["c"], Equals, 2) +} + +// Decoding into a generic map, rather than a struct, takes a different path in +// the decoder (mapping vs mappingStruct), so it needs its own coverage. +func (s *S) TestDuplicateKeys_IntoGenericMap(c *C) { + dec := yaml.NewDecoder(bytes.NewBufferString(dupSrc)) + dec.AllowDuplicateKeys(true) + var out any + c.Assert(dec.Decode(&out), IsNil) + info := out.(map[string]any)["info"].(map[string]any) + c.Assert(info["title"], Equals, "b") +} diff --git a/yaml.go b/yaml.go index f8095e30..fbe71267 100644 --- a/yaml.go +++ b/yaml.go @@ -89,11 +89,12 @@ func Unmarshal(in []byte, out interface{}) (err error) { // A Decoder reads and decodes YAML values from an input stream. type Decoder struct { - parser *parser - knownFields bool - origin bool - file string - disableTimestamps bool + parser *parser + knownFields bool + origin bool + file string + disableTimestamps bool + allowDuplicateKeys bool } // NewDecoder returns a new decoder that reads from r. @@ -106,6 +107,21 @@ func NewDecoder(r io.Reader) *Decoder { } } +// AllowDuplicateKeys controls what happens when a mapping repeats a key. +// +// By default a repeated key is an error, which is YAML's rule and this +// package's long-standing behaviour. Enabling this makes the last occurrence +// win instead, which is what encoding/json does with a repeated JSON object +// name (RFC 8259 says names SHOULD be unique, so the handling is left to the +// implementation). +// +// It exists for callers that decode both YAML and JSON into the same types and +// need one answer for both, rather than a document loading or failing +// depending on which format it happens to be written in. +func (dec *Decoder) AllowDuplicateKeys(enable bool) { + dec.allowDuplicateKeys = enable +} + // KnownFields ensures that the keys in decoded mappings to // exist as fields in the struct being decoded into. func (dec *Decoder) KnownFields(enable bool) { @@ -143,6 +159,7 @@ func (dec *Decoder) DisableTimestamps(disable bool) { func (dec *Decoder) Decode(v interface{}) (err error) { d := newDecoder() d.knownFields = dec.knownFields + d.uniqueKeys = !dec.allowDuplicateKeys d.origin = dec.origin d.file = dec.file d.disableTimestamps = dec.disableTimestamps