diff --git a/doc/godebug.md b/doc/godebug.md index d19de2374aaa75..03618ed9f4170e 100644 --- a/doc/godebug.md +++ b/doc/godebug.md @@ -151,6 +151,18 @@ see the [runtime documentation](/pkg/runtime#hdr-Environment_Variables) and the [go command documentation](/cmd/go#hdr-Build_and_test_caching). ### Go 1.24 +Go 1.24 made JSON marshaling consistent: custom marshalers ([`MarshalJSON`](/pkg/encoding/json#Marshaler) and [`MarshalText`](/pkg/encoding#TextMarshaler)) +are now always called when appropriate no matter if their receivers are pointers or values +even if the related data fields are non-addressable. +This behavior can be reverted with the [`jsoninconsistentmarshal` setting](/pkg/encoding/json/#Marshal). + +Go 1.24 made XML marshaling consistent: custom marshalers ([`MarshalXML`](/pkg/encoding/xml#Marshaler), +[`MarshalXMLAttr`](/pkg/encoding/xml#MarshalerAttr), [`MarshalText`](/pkg/encoding#TextMarshaler)) +are now always called when appropriate no matter if their receivers are pointers or values +even if the related data fields are non-addressable. Also, [`MarshalXMLAttr`](/pkg/encoding/xml#MarshalerAttr) +and [`MarshalText`](/pkg/encoding#TextMarshaler) are now called when appropriate for struct fields +marked as attribute/CDATA/chardata even if the field type is an interface. +This behavior can be reverted with the [`xmlinconsistentmarshal` setting](/pkg/encoding/xml/#Marshal). Go 1.24 changed the global [`math/rand.Seed`](/pkg/math/rand/#Seed) to be a no-op. This setting is controlled by the `randseednop` setting. diff --git a/src/encoding/json/encode.go b/src/encoding/json/encode.go index 988de716124862..e09d7403b9055e 100644 --- a/src/encoding/json/encode.go +++ b/src/encoding/json/encode.go @@ -16,6 +16,7 @@ import ( "encoding" "encoding/base64" "fmt" + "internal/godebug" "math" "reflect" "slices" @@ -157,6 +158,13 @@ import ( // JSON cannot represent cyclic data structures and Marshal does not // handle them. Passing cyclic structures to Marshal will result in // an error. +// +// Before Go 1.24, the marshaling was inconsistent: custom marshalers +// (MarshalJSON and MarshalText methods) defined with pointer receivers +// were not called for non-addressable values. As of Go 1.24, the marshaling is consistent. +// +// The GODEBUG setting jsoninconsistentmarshal=1 restores pre-Go 1.24 +// inconsistent marshaling. func Marshal(v any) ([]byte, error) { e := newEncodeState() defer encodeStatePool.Put(e) @@ -451,7 +459,13 @@ func marshalerEncoder(e *encodeState, v reflect.Value, opts encOpts) { } func addrMarshalerEncoder(e *encodeState, v reflect.Value, opts encOpts) { - va := v.Addr() + var va reflect.Value + if v.CanAddr() { + va = v.Addr() + } else { + va = reflect.New(v.Type()) + va.Elem().Set(v) + } if va.IsNil() { e.WriteString("null") return @@ -487,7 +501,13 @@ func textMarshalerEncoder(e *encodeState, v reflect.Value, opts encOpts) { } func addrTextMarshalerEncoder(e *encodeState, v reflect.Value, opts encOpts) { - va := v.Addr() + var va reflect.Value + if v.CanAddr() { + va = v.Addr() + } else { + va = reflect.New(v.Type()) + va.Elem().Set(v) + } if va.IsNil() { e.WriteString("null") return @@ -897,10 +917,13 @@ type condAddrEncoder struct { canAddrEnc, elseEnc encoderFunc } +var jsoninconsistentmarshal = godebug.New("jsoninconsistentmarshal") + func (ce condAddrEncoder) encode(e *encodeState, v reflect.Value, opts encOpts) { - if v.CanAddr() { + if v.CanAddr() || jsoninconsistentmarshal.Value() != "1" { ce.canAddrEnc(e, v, opts) } else { + jsoninconsistentmarshal.IncNonDefault() ce.elseEnc(e, v, opts) } } diff --git a/src/encoding/json/encode_test.go b/src/encoding/json/encode_test.go index 23a14d0b172927..325866ff97a092 100644 --- a/src/encoding/json/encode_test.go +++ b/src/encoding/json/encode_test.go @@ -10,9 +10,11 @@ import ( "fmt" "log" "math" + "os" "reflect" "regexp" "runtime/debug" + "runtime/metrics" "strconv" "testing" ) @@ -1219,3 +1221,247 @@ func TestIssue63379(t *testing.T) { } } } + +type structWithMarshalJSON struct{ v int } + +func (s *structWithMarshalJSON) MarshalJSON() ([]byte, error) { + return []byte(fmt.Sprintf(`"marshalled(%d)"`, s.v)), nil +} + +var _ = Marshaler(&structWithMarshalJSON{}) + +type embedder struct { + V interface{} +} + +type structWithMarshalText struct{ v int } + +func (s *structWithMarshalText) MarshalText() ([]byte, error) { + return []byte(fmt.Sprintf("marshalled(%d)", s.v)), nil +} + +var _ = encoding.TextMarshaler(&structWithMarshalText{}) + +func TestMarshalJSONWithPointerMarshalers(t *testing.T) { + for _, test := range []struct { + name string + jsoninconsistentmarshal bool + v interface{} + expected string + expectedOldBehaviorCount uint64 + expectedError string + }{ + // MarshalJSON + {name: "a value with MarshalJSON", v: structWithMarshalJSON{v: 1}, expected: `"marshalled(1)"`}, + {name: "pointer to a value with MarshalJSON", v: &structWithMarshalJSON{v: 1}, expected: `"marshalled(1)"`}, + { + name: "a map with a value with MarshalJSON", + v: map[string]interface{}{"v": structWithMarshalJSON{v: 1}}, + expected: `{"v":"marshalled(1)"}`, + }, + { + name: "a map with a pointer to a value with MarshalJSON", + v: map[string]interface{}{"v": &structWithMarshalJSON{v: 1}}, + expected: `{"v":"marshalled(1)"}`, + }, + { + name: "a slice of maps with a value with MarshalJSON", + v: []map[string]interface{}{{"v": structWithMarshalJSON{v: 1}}}, + expected: `[{"v":"marshalled(1)"}]`, + }, + { + name: "a slice of maps with a pointer to a value with MarshalJSON", + v: []map[string]interface{}{{"v": &structWithMarshalJSON{v: 1}}}, + expected: `[{"v":"marshalled(1)"}]`, + }, + { + name: "a struct with a value with MarshalJSON", + v: embedder{V: structWithMarshalJSON{v: 1}}, + expected: `{"V":"marshalled(1)"}`, + }, + { + name: "a slice of structs with a value with MarshalJSON", + v: []embedder{{V: structWithMarshalJSON{v: 1}}}, + expected: `[{"V":"marshalled(1)"}]`, + }, + { + name: "a value with MarshalJSON (only addressable)", + jsoninconsistentmarshal: true, + v: structWithMarshalJSON{v: 1}, + expected: `{}`, + expectedOldBehaviorCount: 1, + }, + { + name: "pointer to a value with MarshalJSON (only addressable)", + jsoninconsistentmarshal: true, + v: &structWithMarshalJSON{v: 1}, + expected: `"marshalled(1)"`, + }, + { + name: "a map with a value with MarshalJSON (only addressable)", + jsoninconsistentmarshal: true, + v: map[string]interface{}{"v": structWithMarshalJSON{v: 1}}, + expected: `{"v":{}}`, + expectedOldBehaviorCount: 1, + }, + { + name: "a map with a pointer to a value with MarshalJSON (only addressable)", + jsoninconsistentmarshal: true, + v: map[string]interface{}{"v": &structWithMarshalJSON{v: 1}}, + expected: `{"v":"marshalled(1)"}`, + }, + { + name: "a slice of maps with a value with MarshalJSON (only addressable)", + jsoninconsistentmarshal: true, + v: []map[string]interface{}{{"v": structWithMarshalJSON{v: 1}}}, + expected: `[{"v":{}}]`, + expectedOldBehaviorCount: 1, + }, + { + name: "a slice of maps with a pointer to a value with MarshalJSON (only addressable)", + jsoninconsistentmarshal: true, + v: []map[string]interface{}{{"v": &structWithMarshalJSON{v: 1}}}, + expected: `[{"v":"marshalled(1)"}]`, + }, + { + name: "a struct with a value with MarshalJSON (only addressable)", + jsoninconsistentmarshal: true, + v: embedder{V: structWithMarshalJSON{v: 1}}, + expected: `{"V":{}}`, + expectedOldBehaviorCount: 1, + }, + { + name: "a slice of structs with a value with MarshalJSON (only addressable)", + jsoninconsistentmarshal: true, + v: []embedder{{V: structWithMarshalJSON{v: 1}}}, + expected: `[{"V":{}}]`, + expectedOldBehaviorCount: 1, + }, + { + name: "a slice of structs with a value with MarshalJSON with two elements (only addressable)", + jsoninconsistentmarshal: true, + v: []embedder{{V: structWithMarshalJSON{v: 1}}, {V: structWithMarshalJSON{v: 2}}}, + expected: `[{"V":{}},{"V":{}}]`, + expectedOldBehaviorCount: 2, + }, + // MarshalText + {name: "a value with MarshalText", v: structWithMarshalText{v: 1}, expected: `"marshalled(1)"`}, + {name: "pointer to a value with MarshalText", v: &structWithMarshalText{v: 1}, expected: `"marshalled(1)"`}, + {name: "a map with a value with MarshalText", v: map[string]interface{}{"v": structWithMarshalText{v: 1}}, expected: `{"v":"marshalled(1)"}`}, + { + name: "a map with a pointer to a value with MarshalText", + v: map[string]interface{}{"v": &structWithMarshalText{v: 1}}, + expected: `{"v":"marshalled(1)"}`, + }, + { + name: "a slice of maps with a value with MarshalText", + v: []map[string]interface{}{{"v": structWithMarshalText{v: 1}}}, + expected: `[{"v":"marshalled(1)"}]`, + }, + { + name: "a slice of maps with a pointer to a value with MarshalText", + v: []map[string]interface{}{{"v": &structWithMarshalText{v: 1}}}, + expected: `[{"v":"marshalled(1)"}]`, + }, + { + name: "a struct with a value with MarshalText", + v: embedder{V: structWithMarshalText{v: 1}}, + expected: `{"V":"marshalled(1)"}`, + }, + { + name: "a slice of structs with a value with MarshalText", + v: []embedder{{V: structWithMarshalText{v: 1}}}, + expected: `[{"V":"marshalled(1)"}]`, + }, + { + name: "a value with MarshalText (only addressable)", + jsoninconsistentmarshal: true, + v: structWithMarshalText{v: 1}, + expected: `{}`, + expectedOldBehaviorCount: 1, + }, + { + name: "pointer to a value with MarshalText (only addressable)", + jsoninconsistentmarshal: true, + v: &structWithMarshalText{v: 1}, + expected: `"marshalled(1)"`, + }, + { + name: "a map with a value with MarshalText (only addressable)", + jsoninconsistentmarshal: true, + v: map[string]interface{}{"v": structWithMarshalText{v: 1}}, + expected: `{"v":{}}`, + expectedOldBehaviorCount: 1, + }, + { + name: "a map with a pointer to a value with MarshalText (only addressable)", + jsoninconsistentmarshal: true, + v: map[string]interface{}{"v": &structWithMarshalText{v: 1}}, + expected: `{"v":"marshalled(1)"}`, + }, + { + name: "a slice of maps with a value with MarshalText (only addressable)", + jsoninconsistentmarshal: true, + v: []map[string]interface{}{{"v": structWithMarshalText{v: 1}}}, + expected: `[{"v":{}}]`, + expectedOldBehaviorCount: 1, + }, + { + name: "a slice of maps with a pointer to a value with MarshalText (only addressable)", + jsoninconsistentmarshal: true, + v: []map[string]interface{}{{"v": &structWithMarshalText{v: 1}}}, + expected: `[{"v":"marshalled(1)"}]`, + }, + { + name: "a struct with a value with MarshalText (only addressable)", + jsoninconsistentmarshal: true, + v: embedder{V: structWithMarshalText{v: 1}}, + expected: `{"V":{}}`, + expectedOldBehaviorCount: 1, + }, + { + name: "a slice of structs with a value with MarshalText (only addressable)", + jsoninconsistentmarshal: true, + v: []embedder{{V: structWithMarshalText{v: 1}}}, + expected: `[{"V":{}}]`, + expectedOldBehaviorCount: 1, + }, + } { + test := test + t.Run(test.name, func(t *testing.T) { + const metricName = "/godebug/non-default-behavior/jsoninconsistentmarshal:events" + sample := make([]metrics.Sample, 1) + sample[0].Name = metricName + metrics.Read(sample) + metricOldValue := sample[0].Value.Uint64() + + if test.jsoninconsistentmarshal { + os.Setenv("GODEBUG", "jsoninconsistentmarshal=1") + defer os.Unsetenv("GODEBUG") + } + result, err := Marshal(test.v) + + metrics.Read(sample) + metricNewValue := sample[0].Value.Uint64() + oldBehaviorCount := metricNewValue - metricOldValue + + if oldBehaviorCount != test.expectedOldBehaviorCount { + t.Errorf("The old behavior count is %d, want %d", oldBehaviorCount, test.expectedOldBehaviorCount) + } + + if err != nil { + if test.expectedError != "" { + if err.Error() != test.expectedError { + t.Errorf("Unexpected Marshal error: %s, expected: %s", err.Error(), test.expectedError) + } + return + } + t.Fatalf("Unexpected Marshal error: %v", err) + } + + if string(result) != test.expected { + t.Errorf("Marshal:\n\tgot: %s\n\twant: %s", result, test.expected) + } + }) + } +} diff --git a/src/encoding/xml/marshal.go b/src/encoding/xml/marshal.go index 05b5542dfb4162..a81628dbe75a8c 100644 --- a/src/encoding/xml/marshal.go +++ b/src/encoding/xml/marshal.go @@ -10,6 +10,7 @@ import ( "encoding" "errors" "fmt" + "internal/godebug" "io" "reflect" "strconv" @@ -77,6 +78,15 @@ const ( // See [MarshalIndent] for an example. // // Marshal will return an error if asked to marshal a channel, function, or map. +// +// Before Go 1.24, the marshaling was inconsistent: custom marshalers +// (MarshalXML, MarshalXMLAttr, MarshalText methods) defined with pointer receivers +// were not called for non-addressable values. Also, MarshalXMLAttr and MarshalText +// were not called for struct fields marked as attribute/CDATA/chardata having interface types. +// As of Go 1.24, the marshaling is consistent. +// +// The GODEBUG setting xmlinconsistentmarshal=1 restores pre-Go 1.24 +// inconsistent marshaling. func Marshal(v any) ([]byte, error) { var b bytes.Buffer enc := NewEncoder(&b) @@ -167,7 +177,7 @@ func (enc *Encoder) Indent(prefix, indent string) { // // Encode calls [Encoder.Flush] before returning. func (enc *Encoder) Encode(v any) error { - err := enc.p.marshalValue(reflect.ValueOf(v), nil, nil) + err := enc.p.marshalValue(reflect.ValueOf(v), nil, nil, false) if err != nil { return err } @@ -182,7 +192,7 @@ func (enc *Encoder) Encode(v any) error { // // EncodeElement calls [Encoder.Flush] before returning. func (enc *Encoder) EncodeElement(v any, start StartElement) error { - err := enc.p.marshalValue(reflect.ValueOf(v), nil, &start) + err := enc.p.marshalValue(reflect.ValueOf(v), nil, &start, false) if err != nil { return err } @@ -415,14 +425,15 @@ func (p *printer) popPrefix() { } var ( - marshalerType = reflect.TypeFor[Marshaler]() - marshalerAttrType = reflect.TypeFor[MarshalerAttr]() - textMarshalerType = reflect.TypeFor[encoding.TextMarshaler]() + marshalerType = reflect.TypeFor[Marshaler]() + marshalerAttrType = reflect.TypeFor[MarshalerAttr]() + textMarshalerType = reflect.TypeFor[encoding.TextMarshaler]() + xmlinconsistentmarshal = godebug.New("xmlinconsistentmarshal") ) // marshalValue writes one or more XML elements representing val. // If val was obtained from a struct field, finfo must have its details. -func (p *printer) marshalValue(val reflect.Value, finfo *fieldInfo, startTemplate *StartElement) error { +func (p *printer) marshalValue(val reflect.Value, finfo *fieldInfo, startTemplate *StartElement, incrementedOldBehaviorCounter bool) error { if startTemplate != nil && startTemplate.Name.Local == "" { return fmt.Errorf("xml: EncodeElement of StartElement with missing name") } @@ -451,28 +462,37 @@ func (p *printer) marshalValue(val reflect.Value, finfo *fieldInfo, startTemplat if val.CanInterface() && typ.Implements(marshalerType) { return p.marshalInterface(val.Interface().(Marshaler), defaultStart(typ, finfo, startTemplate)) } - if val.CanAddr() { - pv := val.Addr() - if pv.CanInterface() && pv.Type().Implements(marshalerType) { + + if val.CanInterface() && reflect.PointerTo(typ).Implements(marshalerType) { + pv := addrOrNew(val) + if pv.IsValid() { return p.marshalInterface(pv.Interface().(Marshaler), defaultStart(pv.Type(), finfo, startTemplate)) } + if !incrementedOldBehaviorCounter { + xmlinconsistentmarshal.IncNonDefault() + incrementedOldBehaviorCounter = true + } } // Check for text marshaler. if val.CanInterface() && typ.Implements(textMarshalerType) { return p.marshalTextInterface(val.Interface().(encoding.TextMarshaler), defaultStart(typ, finfo, startTemplate)) } - if val.CanAddr() { - pv := val.Addr() - if pv.CanInterface() && pv.Type().Implements(textMarshalerType) { + if val.CanInterface() && reflect.PointerTo(typ).Implements(textMarshalerType) { + pv := addrOrNew(val) + if pv.IsValid() { return p.marshalTextInterface(pv.Interface().(encoding.TextMarshaler), defaultStart(pv.Type(), finfo, startTemplate)) } + if !incrementedOldBehaviorCounter { + xmlinconsistentmarshal.IncNonDefault() + incrementedOldBehaviorCounter = true + } } // Slices and arrays iterate over the elements. They do not have an enclosing tag. if (kind == reflect.Slice || kind == reflect.Array) && typ.Elem().Kind() != reflect.Uint8 { for i, n := 0, val.Len(); i < n; i++ { - if err := p.marshalValue(val.Index(i), finfo, startTemplate); err != nil { + if err := p.marshalValue(val.Index(i), finfo, startTemplate, incrementedOldBehaviorCounter); err != nil { return err } } @@ -523,6 +543,8 @@ func (p *printer) marshalValue(val reflect.Value, finfo *fieldInfo, startTemplat // Attributes for i := range tinfo.fields { + attrIncrementedOldBehaviorCounter := incrementedOldBehaviorCounter + finfo := &tinfo.fields[i] if finfo.flags&fAttr == 0 { continue @@ -537,8 +559,17 @@ func (p *printer) marshalValue(val reflect.Value, finfo *fieldInfo, startTemplat continue } + if fv.Kind() == reflect.Interface { + if xmlinconsistentmarshal.Value() != "1" { + fv = fv.Elem() + } else if !attrIncrementedOldBehaviorCounter { + xmlinconsistentmarshal.IncNonDefault() + attrIncrementedOldBehaviorCounter = true + } + } + name := Name{Space: finfo.xmlns, Local: finfo.name} - if err := p.marshalAttr(&start, name, fv); err != nil { + if err := p.marshalAttr(&start, name, fv, attrIncrementedOldBehaviorCounter); err != nil { return err } } @@ -554,7 +585,7 @@ func (p *printer) marshalValue(val reflect.Value, finfo *fieldInfo, startTemplat } if val.Kind() == reflect.Struct { - err = p.marshalStruct(tinfo, val) + err = p.marshalStruct(tinfo, val, incrementedOldBehaviorCounter) } else { s, b, err1 := p.marshalSimple(typ, val) if err1 != nil { @@ -577,7 +608,7 @@ func (p *printer) marshalValue(val reflect.Value, finfo *fieldInfo, startTemplat } // marshalAttr marshals an attribute with the given name and value, adding to start.Attr. -func (p *printer) marshalAttr(start *StartElement, name Name, val reflect.Value) error { +func (p *printer) marshalAttr(start *StartElement, name Name, val reflect.Value, incrementedOldBehaviorCount bool) error { if val.CanInterface() && val.Type().Implements(marshalerAttrType) { attr, err := val.Interface().(MarshalerAttr).MarshalXMLAttr(name) if err != nil { @@ -589,9 +620,9 @@ func (p *printer) marshalAttr(start *StartElement, name Name, val reflect.Value) return nil } - if val.CanAddr() { - pv := val.Addr() - if pv.CanInterface() && pv.Type().Implements(marshalerAttrType) { + if val.CanInterface() && reflect.PointerTo(val.Type()).Implements(marshalerAttrType) { + pv := addrOrNew(val) + if pv.IsValid() { attr, err := pv.Interface().(MarshalerAttr).MarshalXMLAttr(name) if err != nil { return err @@ -601,6 +632,10 @@ func (p *printer) marshalAttr(start *StartElement, name Name, val reflect.Value) } return nil } + if !incrementedOldBehaviorCount { + xmlinconsistentmarshal.IncNonDefault() + incrementedOldBehaviorCount = true + } } if val.CanInterface() && val.Type().Implements(textMarshalerType) { @@ -612,9 +647,9 @@ func (p *printer) marshalAttr(start *StartElement, name Name, val reflect.Value) return nil } - if val.CanAddr() { - pv := val.Addr() - if pv.CanInterface() && pv.Type().Implements(textMarshalerType) { + if val.CanInterface() && reflect.PointerTo(val.Type()).Implements(textMarshalerType) { + pv := addrOrNew(val) + if pv.IsValid() { text, err := pv.Interface().(encoding.TextMarshaler).MarshalText() if err != nil { return err @@ -622,6 +657,10 @@ func (p *printer) marshalAttr(start *StartElement, name Name, val reflect.Value) start.Attr = append(start.Attr, Attr{name, string(text)}) return nil } + if !incrementedOldBehaviorCount { + xmlinconsistentmarshal.IncNonDefault() + incrementedOldBehaviorCount = true + } } // Dereference or skip nil pointer, interface values. @@ -637,7 +676,7 @@ func (p *printer) marshalAttr(start *StartElement, name Name, val reflect.Value) if val.Kind() == reflect.Slice && val.Type().Elem().Kind() != reflect.Uint8 { n := val.Len() for i := 0; i < n; i++ { - if err := p.marshalAttr(start, name, val.Index(i)); err != nil { + if err := p.marshalAttr(start, name, val.Index(i), incrementedOldBehaviorCount); err != nil { return err } } @@ -829,9 +868,10 @@ func indirect(vf reflect.Value) reflect.Value { return vf } -func (p *printer) marshalStruct(tinfo *typeInfo, val reflect.Value) error { +func (p *printer) marshalStruct(tinfo *typeInfo, val reflect.Value, incrementedOldBehaviorCount bool) error { s := parentStack{p: p} for i := range tinfo.fields { + fieldIncrementedOldBehaviorCount := incrementedOldBehaviorCount finfo := &tinfo.fields[i] if finfo.flags&fAttr != 0 { continue @@ -852,6 +892,18 @@ func (p *printer) marshalStruct(tinfo *typeInfo, val reflect.Value) error { if err := s.trim(finfo.parents); err != nil { return err } + + if vf.Kind() == reflect.Interface && !vf.IsNil() { + if xmlinconsistentmarshal.Value() != "1" { + vf = vf.Elem() + } else { + if !fieldIncrementedOldBehaviorCount { + xmlinconsistentmarshal.IncNonDefault() + fieldIncrementedOldBehaviorCount = true + } + } + } + if vf.CanInterface() && vf.Type().Implements(textMarshalerType) { data, err := vf.Interface().(encoding.TextMarshaler).MarshalText() if err != nil { @@ -862,9 +914,9 @@ func (p *printer) marshalStruct(tinfo *typeInfo, val reflect.Value) error { } continue } - if vf.CanAddr() { - pv := vf.Addr() - if pv.CanInterface() && pv.Type().Implements(textMarshalerType) { + if vf.CanInterface() && reflect.PointerTo(vf.Type()).Implements(textMarshalerType) { + pv := addrOrNew(vf) + if pv.IsValid() { data, err := pv.Interface().(encoding.TextMarshaler).MarshalText() if err != nil { return err @@ -874,6 +926,10 @@ func (p *printer) marshalStruct(tinfo *typeInfo, val reflect.Value) error { } continue } + if !fieldIncrementedOldBehaviorCount { + xmlinconsistentmarshal.IncNonDefault() + fieldIncrementedOldBehaviorCount = true + } } var scratch [64]byte @@ -976,7 +1032,7 @@ func (p *printer) marshalStruct(tinfo *typeInfo, val reflect.Value) error { } } } - if err := p.marshalValue(vf, finfo, nil); err != nil { + if err := p.marshalValue(vf, finfo, nil, fieldIncrementedOldBehaviorCount); err != nil { return err } } @@ -1129,3 +1185,15 @@ func isEmptyValue(v reflect.Value) bool { } return false } + +func addrOrNew(v reflect.Value) reflect.Value { + if v.CanAddr() { + return v.Addr() + } + if xmlinconsistentmarshal.Value() != "1" { + pv := reflect.New(v.Type()) + pv.Elem().Set(v) + return pv + } + return reflect.Value{} +} diff --git a/src/encoding/xml/marshal_test.go b/src/encoding/xml/marshal_test.go index b8bce7170a60b6..e2bc96c70649c5 100644 --- a/src/encoding/xml/marshal_test.go +++ b/src/encoding/xml/marshal_test.go @@ -6,10 +6,13 @@ package xml import ( "bytes" + "encoding" "errors" "fmt" "io" + "os" "reflect" + "runtime/metrics" "strconv" "strings" "sync" @@ -2589,3 +2592,394 @@ func TestClose(t *testing.T) { }) } } + +type structWithMarshalXML struct{ V int } + +func (s *structWithMarshalXML) MarshalXML(e *Encoder, _ StartElement) error { + _ = e.EncodeToken(StartElement{Name: Name{Local: "marshalled"}}) + _ = e.EncodeToken(CharData(strconv.Itoa(s.V))) + _ = e.EncodeToken(EndElement{Name: Name{Local: "marshalled"}}) + return nil +} + +var _ = Marshaler(&structWithMarshalXML{}) + +type structWithMarshalText struct{ V int } + +func (s *structWithMarshalText) MarshalText() ([]byte, error) { + return []byte(fmt.Sprintf("marshalled(%d)", s.V)), nil +} + +var _ = encoding.TextMarshaler(&structWithMarshalText{}) + +type structWithMarshalXMLAndMarshalText struct{ V int } + +func (s *structWithMarshalXMLAndMarshalText) MarshalXML(e *Encoder, _ StartElement) error { + _ = e.EncodeToken(StartElement{Name: Name{Local: "marshalled"}}) + _ = e.EncodeToken(CharData(strconv.Itoa(s.V))) + _ = e.EncodeToken(EndElement{Name: Name{Local: "marshalled"}}) + return nil +} + +func (s *structWithMarshalXMLAndMarshalText) MarshalText() ([]byte, error) { + return []byte(fmt.Sprintf("marshalled(%d)", s.V)), nil +} + +var ( + _ = Marshaler(&structWithMarshalXMLAndMarshalText{}) + _ = encoding.TextMarshaler(&structWithMarshalXMLAndMarshalText{}) +) + +type structWithMarshalXMLAttr struct{ v int } + +func (s *structWithMarshalXMLAttr) MarshalXMLAttr(name Name) (Attr, error) { + return Attr{Name: Name{Local: "marshalled"}, Value: strconv.Itoa(s.v)}, nil +} + +var _ = MarshalerAttr(&structWithMarshalXMLAttr{}) + +type embedder struct { + V interface{} +} + +type embedderAt struct { + X interface{} `xml:"X,attr"` + T interface{} `xml:"T,attr"` + XP interface{} `xml:"XP,attr"` + XT interface{} `xml:"XT,attr"` +} + +type embedderAtWithMarshalXMLAttr struct { + X structWithMarshalXMLAttr `xml:"X,attr"` +} + +type embedderAtWithMarshalText struct { + T structWithMarshalText `xml:"T,attr"` +} + +type embedderAtWithMarshalXMLAttrPtr struct { + X *structWithMarshalXMLAttr `xml:"X,attr"` +} + +type embedderAtWithMarshalTextPtr struct { + T *structWithMarshalText `xml:"T,attr"` +} + +type typeWithMarshalXMLAttrAndMarshalText string + +func (s *typeWithMarshalXMLAttrAndMarshalText) MarshalXMLAttr(name Name) (Attr, error) { + return Attr{Name: Name{Local: "marshalled"}, Value: fmt.Sprintf("marshalled(%s)", *s)}, nil +} + +func (s *typeWithMarshalXMLAttrAndMarshalText) MarshalText() ([]byte, error) { + return []byte(fmt.Sprintf("marshalled(%s)", *s)), nil +} + +func ptrToTypeWithMarshalXMLAttrAndMarshalText(s typeWithMarshalXMLAttrAndMarshalText) *typeWithMarshalXMLAttrAndMarshalText { + return &s +} + +var ( + _ = MarshalerAttr(ptrToTypeWithMarshalXMLAttrAndMarshalText("")) + _ = encoding.TextMarshaler(ptrToTypeWithMarshalXMLAttrAndMarshalText("")) +) + +type embedderAtWithMarshalXMLAttrAndMarshalText struct { + X typeWithMarshalXMLAttrAndMarshalText `xml:"X,attr"` + Y typeWithMarshalXMLAttrAndMarshalText `xml:"Y,attr"` +} + +type embedderAtWithMarshalXMLAttrAndMarshalTextAndOwnMarshalXML struct { + X typeWithMarshalXMLAttrAndMarshalText `xml:"X,attr"` + Y typeWithMarshalXMLAttrAndMarshalText `xml:"Y,attr"` +} + +func (s *embedderAtWithMarshalXMLAttrAndMarshalTextAndOwnMarshalXML) MarshalXML(e *Encoder, _ StartElement) error { + _ = e.EncodeToken(StartElement{Name: Name{Local: "marshalled"}}) + _ = e.EncodeToken(EndElement{Name: Name{Local: "marshalled"}}) + return nil +} + +var _ = Marshaler(&embedderAtWithMarshalXMLAttrAndMarshalTextAndOwnMarshalXML{}) + +type embedderCharAndCData struct { + Char typeWithMarshalXMLAttrAndMarshalText `xml:",chardata"` + CData typeWithMarshalXMLAttrAndMarshalText `xml:",cdata"` +} + +func (s *embedderCharAndCDataWithOwnMarshalXML) MarshalXML(e *Encoder, _ StartElement) error { + _ = e.EncodeToken(StartElement{Name: Name{Local: "marshalled"}}) + _ = e.EncodeToken(EndElement{Name: Name{Local: "marshalled"}}) + return nil +} + +var _ = Marshaler(&embedderCharAndCDataWithOwnMarshalXML{}) + +type embedderCharAndCDataWithOwnMarshalXML struct { + Char typeWithMarshalXMLAttrAndMarshalText `xml:",chardata"` + CData typeWithMarshalXMLAttrAndMarshalText `xml:",cdata"` +} + +type embedderCharAndCDataViaInterface struct { + Char interface{} `xml:",chardata"` + CData interface{} `xml:",cdata"` +} + +func TestMarshalXMLWithPointerMarshalers(t *testing.T) { + for _, test := range []struct { + name string + xmlinconsistentmarshal bool + v interface{} + expected string + expectedOldBehaviorCount uint64 + expectedError string + }{ + { + name: "a value with MarshalXML", + v: structWithMarshalXML{V: 1}, + expected: `1`, + }, + { + name: "a pointer to a value with MarshalXML", + v: &structWithMarshalXML{V: 1}, + expected: "1", + }, + { + name: "a struct with a value with MarshalXML", + v: embedder{V: structWithMarshalXML{V: 1}}, + expected: "1", + }, + { + name: "a slice of structs with a value with MarshalXML", + v: []embedder{{V: structWithMarshalXML{V: 1}}}, + expected: `1`, + }, + { + name: "a value with MarshalXML (only addressable)", + xmlinconsistentmarshal: true, + v: structWithMarshalXML{V: 1}, + expected: `1`, + expectedOldBehaviorCount: 1, + }, + { + name: "a pointer to a value with MarshalXML (only addressable)", + xmlinconsistentmarshal: true, + v: &structWithMarshalXML{V: 1}, + expected: "1", + }, + { + name: "a struct with a value with MarshalXML (only addressable)", + xmlinconsistentmarshal: true, + v: embedder{V: structWithMarshalXML{V: 1}}, + expected: "1", + expectedOldBehaviorCount: 1, + }, + { + name: "a slice of structs with a value with MarshalXML (only addressable)", + xmlinconsistentmarshal: true, + v: []embedder{{V: structWithMarshalXML{V: 1}}}, + expected: `1`, + expectedOldBehaviorCount: 1, + }, + { + name: "a value with MarshalText", + v: structWithMarshalText{V: 1}, + expected: "marshalled(1)", + }, + { + name: "pointer to a value with MarshalText", + v: &structWithMarshalText{V: 1}, + expected: "marshalled(1)", + }, + { + name: "a struct with a value with MarshalText", + v: embedder{V: structWithMarshalText{V: 1}}, + expected: "marshalled(1)", + }, + { + name: "a slice of structs with a value with MarshalText", + v: []embedder{{V: structWithMarshalText{V: 1}}}, + expected: "marshalled(1)", + }, + { + name: "a struct with a value with MarshalXML and MarshalText", + v: embedder{V: structWithMarshalXMLAndMarshalText{V: 1}}, + expected: "1", + }, + { + name: "a value with MarshalText (only addressable)", + xmlinconsistentmarshal: true, + v: structWithMarshalText{V: 1}, + expected: "1", + expectedOldBehaviorCount: 1, + }, + { + name: "pointer to a value with MarshalText (only addressable)", + xmlinconsistentmarshal: true, + v: &structWithMarshalText{V: 1}, + expected: "marshalled(1)", + }, + { + name: "a struct with a value with MarshalText (only addressable)", + xmlinconsistentmarshal: true, + v: embedder{V: structWithMarshalText{V: 1}}, + expected: "1", + expectedOldBehaviorCount: 1, + }, + { + name: "a slice of structs with a value with MarshalText (only addressable)", + xmlinconsistentmarshal: true, + v: []embedder{{V: structWithMarshalText{V: 1}}}, + expected: "1", + expectedOldBehaviorCount: 1, + }, + { + name: "a struct with a value with MarshalXML and MarshalText (only addressable)", + xmlinconsistentmarshal: true, + v: embedder{V: structWithMarshalXMLAndMarshalText{V: 1}}, + expected: "1", + expectedOldBehaviorCount: 1, + }, + { + name: "a struct with attributes with MarshalXMLAttr and MarshalText", + v: embedderAt{ + X: structWithMarshalXMLAttr{1}, + T: structWithMarshalText{2}, + XP: &structWithMarshalXMLAttr{3}, + XT: &structWithMarshalText{4}, + }, + expected: ``, + }, + { + name: "a struct with attributes with MarshalXMLAttr and MarshalText (only addressable)", + xmlinconsistentmarshal: true, + v: embedderAt{ + X: structWithMarshalXMLAttr{1}, + T: structWithMarshalText{2}, + XP: &structWithMarshalXMLAttr{3}, + XT: &structWithMarshalText{4}, + }, + expectedOldBehaviorCount: 1, + expectedError: "xml: unsupported type: xml.structWithMarshalXMLAttr", + }, + { + name: "a struct with an attribute with MarshalXMLAttr (only addressable)", + xmlinconsistentmarshal: true, + v: embedderAtWithMarshalXMLAttr{X: structWithMarshalXMLAttr{v: 1}}, + expectedOldBehaviorCount: 1, + expectedError: "xml: unsupported type: xml.structWithMarshalXMLAttr", + }, + { + name: "a struct with an attribute with MarshalText (only addressable)", + xmlinconsistentmarshal: true, + v: embedderAtWithMarshalText{T: structWithMarshalText{V: 1}}, + expectedOldBehaviorCount: 1, + expectedError: "xml: unsupported type: xml.structWithMarshalText", + }, + { + name: "a struct with a pointer attribute with MarshalXMLAttr (only addressable)", + xmlinconsistentmarshal: true, + v: embedderAtWithMarshalXMLAttrPtr{X: &structWithMarshalXMLAttr{v: 1}}, + expected: ``, + }, + { + name: "a struct with a pointer attribute with MarshalText (only addressable)", + xmlinconsistentmarshal: true, + v: embedderAtWithMarshalTextPtr{T: &structWithMarshalText{V: 1}}, + expected: ``, + }, + { + name: "a struct with two attributes with MarshalXMLAttr and MarshalText (only addressable)", + xmlinconsistentmarshal: true, + v: embedderAtWithMarshalXMLAttrAndMarshalText{ + X: typeWithMarshalXMLAttrAndMarshalText("value1"), + Y: typeWithMarshalXMLAttrAndMarshalText("value2"), + }, + expectedOldBehaviorCount: 2, + expected: ``, + }, + { + name: "a struct with two attributes with MarshalXMLAttr and MarshalText having its own MarshalXML with a pointer receiver (only addressable)", + xmlinconsistentmarshal: true, + v: embedderAtWithMarshalXMLAttrAndMarshalTextAndOwnMarshalXML{ + X: "value1", Y: "value2", + }, + expectedOldBehaviorCount: 1, + expected: ``, + }, + { + name: "a struct with chardata and cdata with MarshalText", + v: embedderCharAndCData{Char: "value1", CData: "value2"}, + expected: `marshalled(value1)`, + }, + { + name: "a struct with chardata and cdata with MarshalText via interface{}", + v: embedderCharAndCDataViaInterface{ + Char: typeWithMarshalXMLAttrAndMarshalText("value1"), + CData: typeWithMarshalXMLAttrAndMarshalText("value2"), + }, + expected: `marshalled(value1)`, + }, + { + name: "a struct with chardata and cdata with MarshalText (only addressable)", + xmlinconsistentmarshal: true, + v: embedderCharAndCData{Char: "value1", CData: "value2"}, + expected: `value1`, + expectedOldBehaviorCount: 2, + }, + { + name: "a struct with chardata and cdata with MarshalText having its own MarshalXML (only addressable)", + xmlinconsistentmarshal: true, + v: embedderCharAndCDataWithOwnMarshalXML{Char: "value1", CData: "value2"}, + expected: `value1`, + expectedOldBehaviorCount: 1, + }, + { + name: "a struct with chardata and cdata with MarshalText via interface{} (only addressable)", + xmlinconsistentmarshal: true, + v: embedderCharAndCDataViaInterface{ + Char: typeWithMarshalXMLAttrAndMarshalText("value1"), + CData: typeWithMarshalXMLAttrAndMarshalText("value2"), + }, + expected: `value1`, + expectedOldBehaviorCount: 2, + }, + } { + test := test + t.Run(test.name, func(t *testing.T) { + const metricName = "/godebug/non-default-behavior/xmlinconsistentmarshal:events" + sample := make([]metrics.Sample, 1) + sample[0].Name = metricName + metrics.Read(sample) + metricOldValue := sample[0].Value.Uint64() + + if test.xmlinconsistentmarshal { + os.Setenv("GODEBUG", "xmlinconsistentmarshal=1") + defer os.Unsetenv("GODEBUG") + } + result, err := Marshal(test.v) + + metrics.Read(sample) + metricNewValue := sample[0].Value.Uint64() + oldBehaviorCount := metricNewValue - metricOldValue + + if oldBehaviorCount != test.expectedOldBehaviorCount { + t.Errorf("The old behavior count is %d, want %d", oldBehaviorCount, test.expectedOldBehaviorCount) + } + + if err != nil { + if test.expectedError != "" { + if err.Error() != test.expectedError { + t.Errorf("Unexpected Marshal error: %s, expected: %s", err.Error(), test.expectedError) + } + return + } + t.Fatalf("Unexpected Marshal error: %v", err) + } + + if string(result) != test.expected { + t.Errorf("Marshal:\n\tgot: %s\n\twant: %s", result, test.expected) + } + }) + } +} diff --git a/src/internal/godebugs/table.go b/src/internal/godebugs/table.go index f8d30db5a3b4e9..f002115999f43e 100644 --- a/src/internal/godebugs/table.go +++ b/src/internal/godebugs/table.go @@ -38,6 +38,7 @@ var All = []Info{ {Name: "httpmuxgo121", Package: "net/http", Changed: 22, Old: "1"}, {Name: "httpservecontentkeepheaders", Package: "net/http", Changed: 23, Old: "1"}, {Name: "installgoroot", Package: "go/build"}, + {Name: "jsoninconsistentmarshal", Package: "encoding/json"}, {Name: "jstmpllitinterp", Package: "html/template", Opaque: true}, // bug #66217: remove Opaque //{Name: "multipartfiles", Package: "mime/multipart"}, {Name: "multipartmaxheaders", Package: "mime/multipart"}, @@ -62,6 +63,7 @@ var All = []Info{ {Name: "x509sha1", Package: "crypto/x509"}, {Name: "x509usefallbackroots", Package: "crypto/x509"}, {Name: "x509usepolicies", Package: "crypto/x509"}, + {Name: "xmlinconsistentmarshal", Package: "encoding/xml"}, {Name: "zipinsecurepath", Package: "archive/zip"}, } diff --git a/src/runtime/metrics/doc.go b/src/runtime/metrics/doc.go index 906abb4102e674..077ea701c87217 100644 --- a/src/runtime/metrics/doc.go +++ b/src/runtime/metrics/doc.go @@ -280,6 +280,11 @@ Below is the full list of supported metrics, ordered lexicographically. The number of non-default behaviors executed by the go/build package due to a non-default GODEBUG=installgoroot=... setting. + /godebug/non-default-behavior/jsoninconsistentmarshal:events + The number of non-default behaviors executed by + the encoding/json package due to a non-default + GODEBUG=jsoninconsistentmarshal=... setting. + /godebug/non-default-behavior/multipartmaxheaders:events The number of non-default behaviors executed by the mime/multipart package due to a non-default @@ -367,6 +372,11 @@ Below is the full list of supported metrics, ordered lexicographically. package due to a non-default GODEBUG=x509usepolicies=... setting. + /godebug/non-default-behavior/xmlinconsistentmarshal:events + The number of non-default behaviors executed by the encoding/xml + package due to a non-default GODEBUG=xmlinconsistentmarshal=... + setting. + /godebug/non-default-behavior/zipinsecurepath:events The number of non-default behaviors executed by the archive/zip package due to a non-default GODEBUG=zipinsecurepath=...