Skip to content

Commit b497a29

Browse files
dsnetdsnet
committed
encoding/json: fix regression in quoted numbers under goexperiment.jsonv2
The legacy parsing of quoted numbers in v1 was according to the Go grammar for a number, rather than the JSON grammar for a number. The former is a superset of the latter. This is a historical mistake, but usages exist that depend on it. We already have branches for StringifyWithLegacySemantics to handle quoted nulls, so we can expand it to handle this. Fixes golang#75619 Change-Id: Ic07802539b7cbe0e1f53bd0f7e9bb344a8447203 Reviewed-on: https://go-review.googlesource.com/c/go/+/709615 Reviewed-by: Damien Neil <[email protected]> Auto-Submit: Joseph Tsai <[email protected]> LUCI-TryBot-Result: Go LUCI <[email protected]> Reviewed-by: Michael Pratt <[email protected]>
1 parent 48bb7a6 commit b497a29

File tree

4 files changed

+157
-10
lines changed

4 files changed

+157
-10
lines changed

‎src/encoding/json/decode_test.go‎

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1237,6 +1237,62 @@ var unmarshalTests = []struct {
12371237
out: (chan int)(nil),
12381238
err: &UnmarshalTypeError{Value: "number", Type: reflect.TypeFor[chan int](), Offset: 1},
12391239
},
1240+
1241+
// #75619
1242+
{
1243+
CaseName: Name("QuotedInt/GoSyntax"),
1244+
in: `{"X": "-0000123"}`,
1245+
ptr: new(struct {
1246+
X int64 `json:",string"`
1247+
}),
1248+
out: struct {
1249+
X int64 `json:",string"`
1250+
}{-123},
1251+
},
1252+
{
1253+
CaseName: Name("QuotedInt/Invalid"),
1254+
in: `{"X": "123 "}`,
1255+
ptr: new(struct {
1256+
X int64 `json:",string"`
1257+
}),
1258+
err: &UnmarshalTypeError{Value: "number 123 ", Type: reflect.TypeFor[int64](), Field: "X", Offset: int64(len(`{"X": "123 "`))},
1259+
},
1260+
{
1261+
CaseName: Name("QuotedUint/GoSyntax"),
1262+
in: `{"X": "0000123"}`,
1263+
ptr: new(struct {
1264+
X uint64 `json:",string"`
1265+
}),
1266+
out: struct {
1267+
X uint64 `json:",string"`
1268+
}{123},
1269+
},
1270+
{
1271+
CaseName: Name("QuotedUint/Invalid"),
1272+
in: `{"X": "0x123"}`,
1273+
ptr: new(struct {
1274+
X uint64 `json:",string"`
1275+
}),
1276+
err: &UnmarshalTypeError{Value: "number 0x123", Type: reflect.TypeFor[uint64](), Field: "X", Offset: int64(len(`{"X": "0x123"`))},
1277+
},
1278+
{
1279+
CaseName: Name("QuotedFloat/GoSyntax"),
1280+
in: `{"X": "0x1_4p-2"}`,
1281+
ptr: new(struct {
1282+
X float64 `json:",string"`
1283+
}),
1284+
out: struct {
1285+
X float64 `json:",string"`
1286+
}{0x1_4p-2},
1287+
},
1288+
{
1289+
CaseName: Name("QuotedFloat/Invalid"),
1290+
in: `{"X": "1.5e1_"}`,
1291+
ptr: new(struct {
1292+
X float64 `json:",string"`
1293+
}),
1294+
err: &UnmarshalTypeError{Value: "number 1.5e1_", Type: reflect.TypeFor[float64](), Field: "X", Offset: int64(len(`{"X": "1.5e1_"`))},
1295+
},
12401296
}
12411297

12421298
func TestMarshal(t *testing.T) {

‎src/encoding/json/v2/arshal_default.go‎

Lines changed: 42 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -474,10 +474,21 @@ func makeIntArshaler(t reflect.Type) *arshaler {
474474
break
475475
}
476476
val = jsonwire.UnquoteMayCopy(val, flags.IsVerbatim())
477-
if uo.Flags.Get(jsonflags.StringifyWithLegacySemantics) && string(val) == "null" {
478-
if !uo.Flags.Get(jsonflags.MergeWithLegacySemantics) {
479-
va.SetInt(0)
477+
if uo.Flags.Get(jsonflags.StringifyWithLegacySemantics) {
478+
// For historical reasons, v1 parsed a quoted number
479+
// according to the Go syntax and permitted a quoted null.
480+
// See https://go.dev/issue/75619
481+
n, err := strconv.ParseInt(string(val), 10, bits)
482+
if err != nil {
483+
if string(val) == "null" {
484+
if !uo.Flags.Get(jsonflags.MergeWithLegacySemantics) {
485+
va.SetInt(0)
486+
}
487+
return nil
488+
}
489+
return newUnmarshalErrorAfterWithValue(dec, t, errors.Unwrap(err))
480490
}
491+
va.SetInt(n)
481492
return nil
482493
}
483494
fallthrough
@@ -561,10 +572,21 @@ func makeUintArshaler(t reflect.Type) *arshaler {
561572
break
562573
}
563574
val = jsonwire.UnquoteMayCopy(val, flags.IsVerbatim())
564-
if uo.Flags.Get(jsonflags.StringifyWithLegacySemantics) && string(val) == "null" {
565-
if !uo.Flags.Get(jsonflags.MergeWithLegacySemantics) {
566-
va.SetUint(0)
575+
if uo.Flags.Get(jsonflags.StringifyWithLegacySemantics) {
576+
// For historical reasons, v1 parsed a quoted number
577+
// according to the Go syntax and permitted a quoted null.
578+
// See https://go.dev/issue/75619
579+
n, err := strconv.ParseUint(string(val), 10, bits)
580+
if err != nil {
581+
if string(val) == "null" {
582+
if !uo.Flags.Get(jsonflags.MergeWithLegacySemantics) {
583+
va.SetUint(0)
584+
}
585+
return nil
586+
}
587+
return newUnmarshalErrorAfterWithValue(dec, t, errors.Unwrap(err))
567588
}
589+
va.SetUint(n)
568590
return nil
569591
}
570592
fallthrough
@@ -671,10 +693,21 @@ func makeFloatArshaler(t reflect.Type) *arshaler {
671693
if !stringify {
672694
break
673695
}
674-
if uo.Flags.Get(jsonflags.StringifyWithLegacySemantics) && string(val) == "null" {
675-
if !uo.Flags.Get(jsonflags.MergeWithLegacySemantics) {
676-
va.SetFloat(0)
696+
if uo.Flags.Get(jsonflags.StringifyWithLegacySemantics) {
697+
// For historical reasons, v1 parsed a quoted number
698+
// according to the Go syntax and permitted a quoted null.
699+
// See https://go.dev/issue/75619
700+
n, err := strconv.ParseFloat(string(val), bits)
701+
if err != nil {
702+
if string(val) == "null" {
703+
if !uo.Flags.Get(jsonflags.MergeWithLegacySemantics) {
704+
va.SetFloat(0)
705+
}
706+
return nil
707+
}
708+
return newUnmarshalErrorAfterWithValue(dec, t, errors.Unwrap(err))
677709
}
710+
va.SetFloat(n)
678711
return nil
679712
}
680713
if n, err := jsonwire.ConsumeNumber(val); n != len(val) || err != nil {

‎src/encoding/json/v2_decode_test.go‎

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1243,6 +1243,62 @@ var unmarshalTests = []struct {
12431243
out: (chan int)(nil),
12441244
err: &UnmarshalTypeError{Value: "number", Type: reflect.TypeFor[chan int]()},
12451245
},
1246+
1247+
// #75619
1248+
{
1249+
CaseName: Name("QuotedInt/GoSyntax"),
1250+
in: `{"X": "-0000123"}`,
1251+
ptr: new(struct {
1252+
X int64 `json:",string"`
1253+
}),
1254+
out: struct {
1255+
X int64 `json:",string"`
1256+
}{-123},
1257+
},
1258+
{
1259+
CaseName: Name("QuotedInt/Invalid"),
1260+
in: `{"X": "123 "}`,
1261+
ptr: new(struct {
1262+
X int64 `json:",string"`
1263+
}),
1264+
err: &UnmarshalTypeError{Value: "number 123 ", Type: reflect.TypeFor[int64](), Field: "X", Offset: int64(len(`{"X": `))},
1265+
},
1266+
{
1267+
CaseName: Name("QuotedUint/GoSyntax"),
1268+
in: `{"X": "0000123"}`,
1269+
ptr: new(struct {
1270+
X uint64 `json:",string"`
1271+
}),
1272+
out: struct {
1273+
X uint64 `json:",string"`
1274+
}{123},
1275+
},
1276+
{
1277+
CaseName: Name("QuotedUint/Invalid"),
1278+
in: `{"X": "0x123"}`,
1279+
ptr: new(struct {
1280+
X uint64 `json:",string"`
1281+
}),
1282+
err: &UnmarshalTypeError{Value: "number 0x123", Type: reflect.TypeFor[uint64](), Field: "X", Offset: int64(len(`{"X": `))},
1283+
},
1284+
{
1285+
CaseName: Name("QuotedFloat/GoSyntax"),
1286+
in: `{"X": "0x1_4p-2"}`,
1287+
ptr: new(struct {
1288+
X float64 `json:",string"`
1289+
}),
1290+
out: struct {
1291+
X float64 `json:",string"`
1292+
}{0x1_4p-2},
1293+
},
1294+
{
1295+
CaseName: Name("QuotedFloat/Invalid"),
1296+
in: `{"X": "1.5e1_"}`,
1297+
ptr: new(struct {
1298+
X float64 `json:",string"`
1299+
}),
1300+
err: &UnmarshalTypeError{Value: "number 1.5e1_", Type: reflect.TypeFor[float64](), Field: "X", Offset: int64(len(`{"X": `))},
1301+
},
12461302
}
12471303

12481304
func TestMarshal(t *testing.T) {

‎src/encoding/json/v2_options.go‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -506,7 +506,9 @@ func ReportErrorsWithLegacySemantics(v bool) Options {
506506
// When marshaling, such Go values are serialized as their usual
507507
// JSON representation, but quoted within a JSON string.
508508
// When unmarshaling, such Go values must be deserialized from
509-
// a JSON string containing their usual JSON representation.
509+
// a JSON string containing their usual JSON representation or
510+
// Go number representation for that numeric kind.
511+
// Note that the Go number grammar is a superset of the JSON number grammar.
510512
// A JSON null quoted in a JSON string is a valid substitute for JSON null
511513
// while unmarshaling into a Go value that `string` takes effect on.
512514
//

0 commit comments

Comments
 (0)