encoding/json, encoding/xml: add full support for marshalers with pointer receivers

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.

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)
 	}
 }

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)
+			}
+		})
+	}
+}