Skip to content

Commit 78d2393

Browse files
zenovichzenovich
committed
encoding/xml: call MarshalText() defined with pointer receivers for cdata/chardata fields even when they are non-addressable of non-pointer types on marshalling XML; call MarshalXML(), MarshalXMLAttr(), and MarshalText() even if the struct field has an interface type; introduce the GODEBUG setting "xmlinconsistentmarshal" allowing to revert the new consistent XML marshalling
1 parent 928e3d9 commit 78d2393

File tree

5 files changed

+478
-119
lines changed

5 files changed

+478
-119
lines changed

‎doc/godebug.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -150,6 +150,9 @@ for example,
150150
see the [runtime documentation](/pkg/runtime#hdr-Environment_Variables)
151151
and the [go command documentation](/cmd/go#hdr-Build_and_test_caching).
152152

153+
### Go 1.24
154+
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).
155+
153156
### Go 1.23
154157

155158
Go 1.23 changed the channels created by package time to be unbuffered

‎src/encoding/xml/marshal.go

Lines changed: 112 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import (
1010
"encoding"
1111
"errors"
1212
"fmt"
13+
"internal/godebug"
1314
"io"
1415
"reflect"
1516
"strconv"
@@ -77,6 +78,15 @@ const (
7778
// See [MarshalIndent] for an example.
7879
//
7980
// Marshal will return an error if asked to marshal a channel, function, or map.
81+
//
82+
// Before Go 1.24, the marshaling was inconsistent: custom marshalers
83+
// (MarshalXML, MarshalXMLAttr, MarshalText methods) defined with pointer receivers
84+
// were not called for non-addressable values. Also, MarshalXMLAttr and MarshalText
85+
// were not called for struct fields marked as attribute/CDATA/chardata having interface types.
86+
// As of Go 1.24, the marshaling is consistent.
87+
//
88+
// The GODEBUG setting xmlinconsistentmarshal=1 restores pre-Go 1.24
89+
// inconsistent marshaling.
8090
func Marshal(v any) ([]byte, error) {
8191
var b bytes.Buffer
8292
enc := NewEncoder(&b)
@@ -167,7 +177,7 @@ func (enc *Encoder) Indent(prefix, indent string) {
167177
//
168178
// Encode calls [Encoder.Flush] before returning.
169179
func (enc *Encoder) Encode(v any) error {
170-
err := enc.p.marshalValue(reflect.ValueOf(v), nil, nil)
180+
err := enc.p.marshalValue(reflect.ValueOf(v), nil, nil, false)
171181
if err != nil {
172182
return err
173183
}
@@ -182,7 +192,7 @@ func (enc *Encoder) Encode(v any) error {
182192
//
183193
// EncodeElement calls [Encoder.Flush] before returning.
184194
func (enc *Encoder) EncodeElement(v any, start StartElement) error {
185-
err := enc.p.marshalValue(reflect.ValueOf(v), nil, &start)
195+
err := enc.p.marshalValue(reflect.ValueOf(v), nil, &start, false)
186196
if err != nil {
187197
return err
188198
}
@@ -415,14 +425,15 @@ func (p *printer) popPrefix() {
415425
}
416426

417427
var (
418-
marshalerType = reflect.TypeFor[Marshaler]()
419-
marshalerAttrType = reflect.TypeFor[MarshalerAttr]()
420-
textMarshalerType = reflect.TypeFor[encoding.TextMarshaler]()
428+
marshalerType = reflect.TypeFor[Marshaler]()
429+
marshalerAttrType = reflect.TypeFor[MarshalerAttr]()
430+
textMarshalerType = reflect.TypeFor[encoding.TextMarshaler]()
431+
xmlinconsistentmarshal = godebug.New("xmlinconsistentmarshal")
421432
)
422433

423434
// marshalValue writes one or more XML elements representing val.
424435
// If val was obtained from a struct field, finfo must have its details.
425-
func (p *printer) marshalValue(val reflect.Value, finfo *fieldInfo, startTemplate *StartElement) error {
436+
func (p *printer) marshalValue(val reflect.Value, finfo *fieldInfo, startTemplate *StartElement, incrementedOldBehaviorCounter bool) error {
426437
if startTemplate != nil && startTemplate.Name.Local == "" {
427438
return fmt.Errorf("xml: EncodeElement of StartElement with missing name")
428439
}
@@ -452,30 +463,36 @@ func (p *printer) marshalValue(val reflect.Value, finfo *fieldInfo, startTemplat
452463
return p.marshalInterface(val.Interface().(Marshaler), defaultStart(typ, finfo, startTemplate))
453464
}
454465

455-
var pv reflect.Value
456-
if val.CanAddr() {
457-
pv = val.Addr()
458-
} else {
459-
pv = reflect.New(typ)
460-
pv.Elem().Set(val)
461-
}
462-
463-
if pv.CanInterface() && pv.Type().Implements(marshalerType) {
464-
return p.marshalInterface(pv.Interface().(Marshaler), defaultStart(pv.Type(), finfo, startTemplate))
466+
if val.CanInterface() && reflect.PointerTo(typ).Implements(marshalerType) {
467+
pv := addrOrNew(val)
468+
if pv.IsValid() {
469+
return p.marshalInterface(pv.Interface().(Marshaler), defaultStart(pv.Type(), finfo, startTemplate))
470+
}
471+
if !incrementedOldBehaviorCounter {
472+
xmlinconsistentmarshal.IncNonDefault()
473+
incrementedOldBehaviorCounter = true
474+
}
465475
}
466476

467477
// Check for text marshaler.
468478
if val.CanInterface() && typ.Implements(textMarshalerType) {
469479
return p.marshalTextInterface(val.Interface().(encoding.TextMarshaler), defaultStart(typ, finfo, startTemplate))
470480
}
471-
if pv.CanInterface() && pv.Type().Implements(textMarshalerType) {
472-
return p.marshalTextInterface(pv.Interface().(encoding.TextMarshaler), defaultStart(pv.Type(), finfo, startTemplate))
481+
if val.CanInterface() && reflect.PointerTo(typ).Implements(textMarshalerType) {
482+
pv := addrOrNew(val)
483+
if pv.IsValid() {
484+
return p.marshalTextInterface(pv.Interface().(encoding.TextMarshaler), defaultStart(pv.Type(), finfo, startTemplate))
485+
}
486+
if !incrementedOldBehaviorCounter {
487+
xmlinconsistentmarshal.IncNonDefault()
488+
incrementedOldBehaviorCounter = true
489+
}
473490
}
474491

475492
// Slices and arrays iterate over the elements. They do not have an enclosing tag.
476493
if (kind == reflect.Slice || kind == reflect.Array) && typ.Elem().Kind() != reflect.Uint8 {
477494
for i, n := 0, val.Len(); i < n; i++ {
478-
if err := p.marshalValue(val.Index(i), finfo, startTemplate); err != nil {
495+
if err := p.marshalValue(val.Index(i), finfo, startTemplate, incrementedOldBehaviorCounter); err != nil {
479496
return err
480497
}
481498
}
@@ -526,6 +543,8 @@ func (p *printer) marshalValue(val reflect.Value, finfo *fieldInfo, startTemplat
526543

527544
// Attributes
528545
for i := range tinfo.fields {
546+
attrIncrementedOldBehaviorCounter := incrementedOldBehaviorCounter
547+
529548
finfo := &tinfo.fields[i]
530549
if finfo.flags&fAttr == 0 {
531550
continue
@@ -540,8 +559,17 @@ func (p *printer) marshalValue(val reflect.Value, finfo *fieldInfo, startTemplat
540559
continue
541560
}
542561

562+
if fv.Kind() == reflect.Interface {
563+
if xmlinconsistentmarshal.Value() != "1" {
564+
fv = fv.Elem()
565+
} else if !attrIncrementedOldBehaviorCounter {
566+
xmlinconsistentmarshal.IncNonDefault()
567+
attrIncrementedOldBehaviorCounter = true
568+
}
569+
}
570+
543571
name := Name{Space: finfo.xmlns, Local: finfo.name}
544-
if err := p.marshalAttr(&start, name, fv); err != nil {
572+
if err := p.marshalAttr(&start, name, fv, attrIncrementedOldBehaviorCounter); err != nil {
545573
return err
546574
}
547575
}
@@ -557,7 +585,7 @@ func (p *printer) marshalValue(val reflect.Value, finfo *fieldInfo, startTemplat
557585
}
558586

559587
if val.Kind() == reflect.Struct {
560-
err = p.marshalStruct(tinfo, val)
588+
err = p.marshalStruct(tinfo, val, incrementedOldBehaviorCounter)
561589
} else {
562590
s, b, err1 := p.marshalSimple(typ, val)
563591
if err1 != nil {
@@ -580,7 +608,7 @@ func (p *printer) marshalValue(val reflect.Value, finfo *fieldInfo, startTemplat
580608
}
581609

582610
// marshalAttr marshals an attribute with the given name and value, adding to start.Attr.
583-
func (p *printer) marshalAttr(start *StartElement, name Name, val reflect.Value) error {
611+
func (p *printer) marshalAttr(start *StartElement, name Name, val reflect.Value, incrementedOldBehaviorCount bool) error {
584612
if val.CanInterface() && val.Type().Implements(marshalerAttrType) {
585613
attr, err := val.Interface().(MarshalerAttr).MarshalXMLAttr(name)
586614
if err != nil {
@@ -592,23 +620,22 @@ func (p *printer) marshalAttr(start *StartElement, name Name, val reflect.Value)
592620
return nil
593621
}
594622

595-
var pv reflect.Value
596-
if val.CanAddr() {
597-
pv = val.Addr()
598-
} else {
599-
pv = reflect.New(val.Type())
600-
pv.Elem().Set(val)
601-
}
602-
603-
if pv.CanInterface() && pv.Type().Implements(marshalerAttrType) {
604-
attr, err := pv.Interface().(MarshalerAttr).MarshalXMLAttr(name)
605-
if err != nil {
606-
return err
623+
if val.CanInterface() && reflect.PointerTo(val.Type()).Implements(marshalerAttrType) {
624+
pv := addrOrNew(val)
625+
if pv.IsValid() {
626+
attr, err := pv.Interface().(MarshalerAttr).MarshalXMLAttr(name)
627+
if err != nil {
628+
return err
629+
}
630+
if attr.Name.Local != "" {
631+
start.Attr = append(start.Attr, attr)
632+
}
633+
return nil
607634
}
608-
if attr.Name.Local != "" {
609-
start.Attr = append(start.Attr, attr)
635+
if !incrementedOldBehaviorCount {
636+
xmlinconsistentmarshal.IncNonDefault()
637+
incrementedOldBehaviorCount = true
610638
}
611-
return nil
612639
}
613640

614641
if val.CanInterface() && val.Type().Implements(textMarshalerType) {
@@ -620,13 +647,20 @@ func (p *printer) marshalAttr(start *StartElement, name Name, val reflect.Value)
620647
return nil
621648
}
622649

623-
if pv.CanInterface() && pv.Type().Implements(textMarshalerType) {
624-
text, err := pv.Interface().(encoding.TextMarshaler).MarshalText()
625-
if err != nil {
626-
return err
650+
if val.CanInterface() && reflect.PointerTo(val.Type()).Implements(textMarshalerType) {
651+
pv := addrOrNew(val)
652+
if pv.IsValid() {
653+
text, err := pv.Interface().(encoding.TextMarshaler).MarshalText()
654+
if err != nil {
655+
return err
656+
}
657+
start.Attr = append(start.Attr, Attr{name, string(text)})
658+
return nil
659+
}
660+
if !incrementedOldBehaviorCount {
661+
xmlinconsistentmarshal.IncNonDefault()
662+
incrementedOldBehaviorCount = true
627663
}
628-
start.Attr = append(start.Attr, Attr{name, string(text)})
629-
return nil
630664
}
631665

632666
// Dereference or skip nil pointer, interface values.
@@ -642,7 +676,7 @@ func (p *printer) marshalAttr(start *StartElement, name Name, val reflect.Value)
642676
if val.Kind() == reflect.Slice && val.Type().Elem().Kind() != reflect.Uint8 {
643677
n := val.Len()
644678
for i := 0; i < n; i++ {
645-
if err := p.marshalAttr(start, name, val.Index(i)); err != nil {
679+
if err := p.marshalAttr(start, name, val.Index(i), incrementedOldBehaviorCount); err != nil {
646680
return err
647681
}
648682
}
@@ -834,9 +868,10 @@ func indirect(vf reflect.Value) reflect.Value {
834868
return vf
835869
}
836870

837-
func (p *printer) marshalStruct(tinfo *typeInfo, val reflect.Value) error {
871+
func (p *printer) marshalStruct(tinfo *typeInfo, val reflect.Value, incrementedOldBehaviorCount bool) error {
838872
s := parentStack{p: p}
839873
for i := range tinfo.fields {
874+
fieldIncrementedOldBehaviorCount := incrementedOldBehaviorCount
840875
finfo := &tinfo.fields[i]
841876
if finfo.flags&fAttr != 0 {
842877
continue
@@ -857,6 +892,18 @@ func (p *printer) marshalStruct(tinfo *typeInfo, val reflect.Value) error {
857892
if err := s.trim(finfo.parents); err != nil {
858893
return err
859894
}
895+
896+
if vf.Kind() == reflect.Interface && !vf.IsNil() {
897+
if xmlinconsistentmarshal.Value() != "1" {
898+
vf = vf.Elem()
899+
} else {
900+
if !fieldIncrementedOldBehaviorCount {
901+
xmlinconsistentmarshal.IncNonDefault()
902+
fieldIncrementedOldBehaviorCount = true
903+
}
904+
}
905+
}
906+
860907
if vf.CanInterface() && vf.Type().Implements(textMarshalerType) {
861908
data, err := vf.Interface().(encoding.TextMarshaler).MarshalText()
862909
if err != nil {
@@ -867,9 +914,9 @@ func (p *printer) marshalStruct(tinfo *typeInfo, val reflect.Value) error {
867914
}
868915
continue
869916
}
870-
if vf.CanAddr() {
871-
pv := vf.Addr()
872-
if pv.CanInterface() && pv.Type().Implements(textMarshalerType) {
917+
if vf.CanInterface() && reflect.PointerTo(vf.Type()).Implements(textMarshalerType) {
918+
pv := addrOrNew(vf)
919+
if pv.IsValid() {
873920
data, err := pv.Interface().(encoding.TextMarshaler).MarshalText()
874921
if err != nil {
875922
return err
@@ -879,6 +926,10 @@ func (p *printer) marshalStruct(tinfo *typeInfo, val reflect.Value) error {
879926
}
880927
continue
881928
}
929+
if !fieldIncrementedOldBehaviorCount {
930+
xmlinconsistentmarshal.IncNonDefault()
931+
fieldIncrementedOldBehaviorCount = true
932+
}
882933
}
883934

884935
var scratch [64]byte
@@ -981,7 +1032,7 @@ func (p *printer) marshalStruct(tinfo *typeInfo, val reflect.Value) error {
9811032
}
9821033
}
9831034
}
984-
if err := p.marshalValue(vf, finfo, nil); err != nil {
1035+
if err := p.marshalValue(vf, finfo, nil, fieldIncrementedOldBehaviorCount); err != nil {
9851036
return err
9861037
}
9871038
}
@@ -1134,3 +1185,15 @@ func isEmptyValue(v reflect.Value) bool {
11341185
}
11351186
return false
11361187
}
1188+
1189+
func addrOrNew(v reflect.Value) reflect.Value {
1190+
if v.CanAddr() {
1191+
return v.Addr()
1192+
}
1193+
if xmlinconsistentmarshal.Value() != "1" {
1194+
pv := reflect.New(v.Type())
1195+
pv.Elem().Set(v)
1196+
return pv
1197+
}
1198+
return reflect.Value{}
1199+
}

0 commit comments

Comments
 (0)