From 925769159101bc9a61cb63287133cdb222c37fc1 Mon Sep 17 00:00:00 2001 From: Joe Tsai Date: Wed, 20 Nov 2024 05:47:14 -0800 Subject: [PATCH] Add support for legacy string tag option behavior In v1, the `string` tag option would stringify bools and strings. Add an option to support this behavior in v2. --- arshal_default.go | 80 +++++++++- arshal_test.go | 288 ++++++++++++++++++++++++++++++++++++ internal/jsonflags/flags.go | 1 + v1/options.go | 20 +++ 4 files changed, 382 insertions(+), 7 deletions(-) diff --git a/arshal_default.go b/arshal_default.go index d491500..c8ba351 100644 --- a/arshal_default.go +++ b/arshal_default.go @@ -126,7 +126,7 @@ func makeBoolArshaler(t reflect.Type) *arshaler { } // Optimize for marshaling without preceding whitespace. - if optimizeCommon && !xe.Flags.Get(jsonflags.AnyWhitespace) && !xe.Tokens.Last.NeedObjectName() { + if optimizeCommon && !xe.Flags.Get(jsonflags.AnyWhitespace) && !mo.Flags.Get(jsonflags.StringifyBoolsAndStrings) && !xe.Tokens.Last.NeedObjectName() { xe.Buf = strconv.AppendBool(xe.Tokens.MayAppendDelim(xe.Buf, 't'), va.Bool()) xe.Tokens.Last.Increment() if xe.NeedFlush() { @@ -135,6 +135,13 @@ func makeBoolArshaler(t reflect.Type) *arshaler { return nil } + if mo.Flags.Get(jsonflags.StringifyBoolsAndStrings) { + if va.Bool() { + return enc.WriteToken(jsontext.String("true")) + } else { + return enc.WriteToken(jsontext.String("false")) + } + } return enc.WriteToken(jsontext.Bool(va.Bool())) } fncs.unmarshal = func(dec *jsontext.Decoder, va addressableValue, uo *jsonopts.Struct) error { @@ -152,8 +159,22 @@ func makeBoolArshaler(t reflect.Type) *arshaler { va.SetBool(false) return nil case 't', 'f': - va.SetBool(tok.Bool()) - return nil + if !uo.Flags.Get(jsonflags.StringifyBoolsAndStrings) { + va.SetBool(tok.Bool()) + return nil + } + case '"': + if uo.Flags.Get(jsonflags.StringifyBoolsAndStrings) { + switch tok.String() { + case "true": + va.SetBool(true) + case "false": + va.SetBool(false) + default: + return &SemanticError{action: "unmarshal", JSONKind: k, GoType: t, Err: fmt.Errorf("cannot parse %q as bool", tok.String())} + } + return nil + } } return &SemanticError{action: "unmarshal", JSONKind: k, GoType: t} } @@ -170,7 +191,7 @@ func makeStringArshaler(t reflect.Type) *arshaler { // Optimize for marshaling without preceding whitespace or string escaping. s := va.String() - if optimizeCommon && !xe.Flags.Get(jsonflags.AnyWhitespace) && !xe.Tokens.Last.NeedObjectName() && !jsonwire.NeedEscape(s) { + if optimizeCommon && !xe.Flags.Get(jsonflags.AnyWhitespace) && !mo.Flags.Get(jsonflags.StringifyBoolsAndStrings) && !xe.Tokens.Last.NeedObjectName() && !jsonwire.NeedEscape(s) { b := xe.Buf b = xe.Tokens.MayAppendDelim(b, '"') b = append(b, '"') @@ -184,6 +205,14 @@ func makeStringArshaler(t reflect.Type) *arshaler { return nil } + if mo.Flags.Get(jsonflags.StringifyBoolsAndStrings) { + b, err := jsontext.AppendQuote(nil, s) // only fails for invalid UTF-8 + q, _ := jsontext.AppendQuote(nil, b) // cannot fail since b is valid UTF-8 + if err != nil && !xe.Flags.Get(jsonflags.AllowInvalidUTF8) { + return err + } + return enc.WriteValue(q) + } return enc.WriteToken(jsontext.String(s)) } fncs.unmarshal = func(dec *jsontext.Decoder, va addressableValue, uo *jsonopts.Struct) error { @@ -203,6 +232,12 @@ func makeStringArshaler(t reflect.Type) *arshaler { return nil case '"': val = jsonwire.UnquoteMayCopy(val, flags.IsVerbatim()) + if uo.Flags.Get(jsonflags.StringifyBoolsAndStrings) { + val, err = jsontext.AppendUnquote(nil, val) + if err != nil { + return &SemanticError{action: "unmarshal", JSONKind: '"', GoType: va.Type(), Err: err} + } + } if xd.StringCache == nil { xd.StringCache = new(stringCache) } @@ -1014,7 +1049,13 @@ func makeStructArshaler(t reflect.Type) *arshaler { // Write the object member value. flagsOriginal := mo.Flags if f.string { - mo.Flags.Set(jsonflags.StringifyNumbers | 1) + if mo.Flags.Get(jsonflags.StringifyWithLegacySemantics) { + if canLegacyStringify(f.typ) { + mo.Flags.Set(jsonflags.StringifyNumbers | jsonflags.StringifyBoolsAndStrings | 1) + } + } else { + mo.Flags.Set(jsonflags.StringifyNumbers | 1) + } } if f.format != "" { mo.FormatDepth = xe.Tokens.Depth() @@ -1153,7 +1194,13 @@ func makeStructArshaler(t reflect.Type) *arshaler { } flagsOriginal := uo.Flags if f.string { - uo.Flags.Set(jsonflags.StringifyNumbers | 1) + if uo.Flags.Get(jsonflags.StringifyWithLegacySemantics) { + if canLegacyStringify(f.typ) { + uo.Flags.Set(jsonflags.StringifyNumbers | jsonflags.StringifyBoolsAndStrings | 1) + } + } else { + uo.Flags.Set(jsonflags.StringifyNumbers | 1) + } } if f.format != "" { uo.FormatDepth = xd.Tokens.Depth() @@ -1224,6 +1271,25 @@ func isLegacyEmpty(v addressableValue) bool { return false } +// canLegacyStringify reports whether t can be stringified according to v1, +// where t is a bool, string, or number (or unnamed pointer to such). +// In v1, the `string` option does not apply recursively to nested types within +// a composite Go type (e.g., a array, slice, struct, map, or interface). +func canLegacyStringify(t reflect.Type) bool { + // Based on encoding/json.typeFields#L1126-L1143@v1.23.0 + if t.Name() == "" && t.Kind() == reflect.Ptr { + t = t.Elem() + } + switch t.Kind() { + case reflect.Bool, reflect.String, + reflect.Int, reflect.Int8, reflect.Int16, reflect.Int32, reflect.Int64, + reflect.Uint, reflect.Uint8, reflect.Uint16, reflect.Uint32, reflect.Uint64, reflect.Uintptr, + reflect.Float32, reflect.Float64: + return true + } + return false +} + func makeSliceArshaler(t reflect.Type) *arshaler { var fncs arshaler var ( @@ -1527,7 +1593,7 @@ func makeInterfaceArshaler(t reflect.Type) *arshaler { } // Optimize for the any type if there are no special options. if optimizeCommon && - t == anyType && !mo.Flags.Get(jsonflags.StringifyNumbers) && mo.Format == "" && + t == anyType && !mo.Flags.Get(jsonflags.StringifyNumbers) && !mo.Flags.Get(jsonflags.StringifyBoolsAndStrings) && mo.Format == "" && (mo.Marshalers == nil || !mo.Marshalers.(*Marshalers).fromAny) { return marshalValueAny(enc, va.Elem().Interface(), mo) } diff --git a/arshal_test.go b/arshal_test.go index 3b36d92..ffaabf8 100644 --- a/arshal_test.go +++ b/arshal_test.go @@ -693,6 +693,11 @@ func TestMarshal(t *testing.T) { opts: []Options{StringifyNumbers(true)}, in: []bool{false, true}, want: `[false,true]`, + }, { + name: jsontest.Name("Bools/StringifiedBool"), + opts: []Options{jsonflags.StringifyBoolsAndStrings | 1}, + in: []bool{false, true}, + want: `["false","true"]`, }, { name: jsontest.Name("Bools/IgnoreInvalidFormat"), opts: []Options{invalidFormatOption}, @@ -706,6 +711,11 @@ func TestMarshal(t *testing.T) { name: jsontest.Name("Strings/Named"), in: []namedString{"", "hello", "世界"}, want: `["","hello","世界"]`, + }, { + name: jsontest.Name("Strings/StringifiedBool"), + opts: []Options{jsonflags.StringifyBoolsAndStrings | 1}, + in: []string{"", "hello", "世界"}, + want: `["\"\"","\"hello\"","\"世界\""]`, }, { name: jsontest.Name("Strings/IgnoreInvalidFormat"), opts: []Options{invalidFormatOption}, @@ -1391,6 +1401,152 @@ func TestMarshal(t *testing.T) { "Interface": null }, "Interface": null +}`, + }, { + name: jsontest.Name("Structs/LegacyStringified"), + opts: []Options{jsontext.Multiline(true), jsonflags.StringifyWithLegacySemantics | 1}, + in: structStringifiedAll{ + Bool: true, // should be stringified + String: "hello", // should be stringified + Bytes: []byte{1, 2, 3}, + Int: -64, // should be stringified + Uint: +64, // should be stringified + Float: 3.14159, // should be stringified + Map: map[string]string{"key": "value"}, + StructScalars: structScalars{ + Bool: true, + String: "hello", + Bytes: []byte{1, 2, 3}, + Int: -64, + Uint: +64, + Float: 3.14159, + }, + StructMaps: structMaps{ + MapBool: map[string]bool{"": true}, + MapString: map[string]string{"": "hello"}, + MapBytes: map[string][]byte{"": {1, 2, 3}}, + MapInt: map[string]int64{"": -64}, + MapUint: map[string]uint64{"": +64}, + MapFloat: map[string]float64{"": 3.14159}, + }, + StructSlices: structSlices{ + SliceBool: []bool{true}, + SliceString: []string{"hello"}, + SliceBytes: [][]byte{{1, 2, 3}}, + SliceInt: []int64{-64}, + SliceUint: []uint64{+64}, + SliceFloat: []float64{3.14159}, + }, + Slice: []string{"fizz", "buzz"}, + Array: [1]string{"goodbye"}, + Pointer: new(structStringifiedAll), // should be stringified + Interface: (*structStringifiedAll)(nil), + }, + want: `{ + "Bool": "true", + "String": "\"hello\"", + "Bytes": "AQID", + "Int": "-64", + "Uint": "64", + "Float": "3.14159", + "Map": { + "key": "value" + }, + "StructScalars": { + "Bool": true, + "String": "hello", + "Bytes": "AQID", + "Int": -64, + "Uint": 64, + "Float": 3.14159 + }, + "StructMaps": { + "MapBool": { + "": true + }, + "MapString": { + "": "hello" + }, + "MapBytes": { + "": "AQID" + }, + "MapInt": { + "": -64 + }, + "MapUint": { + "": 64 + }, + "MapFloat": { + "": 3.14159 + } + }, + "StructSlices": { + "SliceBool": [ + true + ], + "SliceString": [ + "hello" + ], + "SliceBytes": [ + "AQID" + ], + "SliceInt": [ + -64 + ], + "SliceUint": [ + 64 + ], + "SliceFloat": [ + 3.14159 + ] + }, + "Slice": [ + "fizz", + "buzz" + ], + "Array": [ + "goodbye" + ], + "Pointer": { + "Bool": "false", + "String": "\"\"", + "Bytes": "", + "Int": "0", + "Uint": "0", + "Float": "0", + "Map": {}, + "StructScalars": { + "Bool": false, + "String": "", + "Bytes": "", + "Int": 0, + "Uint": 0, + "Float": 0 + }, + "StructMaps": { + "MapBool": {}, + "MapString": {}, + "MapBytes": {}, + "MapInt": {}, + "MapUint": {}, + "MapFloat": {} + }, + "StructSlices": { + "SliceBool": [], + "SliceString": [], + "SliceBytes": [], + "SliceInt": [], + "SliceUint": [], + "SliceFloat": [] + }, + "Slice": [], + "Array": [ + "" + ], + "Pointer": null, + "Interface": null + }, + "Interface": null }`, }, { name: jsontest.Name("Structs/OmitZero/Zero"), @@ -4253,6 +4409,32 @@ func TestUnmarshal(t *testing.T) { inVal: addr(true), want: addr(true), wantErr: &SemanticError{action: "unmarshal", JSONKind: '"', GoType: boolType}, + }, { + name: jsontest.Name("Bools/StringifiedBool/True"), + opts: []Options{jsonflags.StringifyBoolsAndStrings | 1}, + inBuf: `"true"`, + inVal: addr(false), + want: addr(true), + }, { + name: jsontest.Name("Bools/StringifiedBool/False"), + opts: []Options{jsonflags.StringifyBoolsAndStrings | 1}, + inBuf: `"false"`, + inVal: addr(true), + want: addr(false), + }, { + name: jsontest.Name("Bools/StringifiedBool/InvalidWhitespace"), + opts: []Options{jsonflags.StringifyBoolsAndStrings | 1}, + inBuf: `"false "`, + inVal: addr(true), + want: addr(true), + wantErr: &SemanticError{action: "unmarshal", JSONKind: '"', GoType: boolType, Err: errors.New("cannot parse \"false \" as bool")}, + }, { + name: jsontest.Name("Bools/StringifiedBool/InvalidBool"), + opts: []Options{jsonflags.StringifyBoolsAndStrings | 1}, + inBuf: `false`, + inVal: addr(true), + want: addr(true), + wantErr: &SemanticError{action: "unmarshal", JSONKind: 'f', GoType: boolType}, }, { name: jsontest.Name("Bools/Invalid/Number"), inBuf: `0`, @@ -4333,6 +4515,26 @@ func TestUnmarshal(t *testing.T) { inBuf: `"hello"`, inVal: addr("goodbye"), want: addr("hello"), + }, { + name: jsontest.Name("Strings/StringifiedString"), + opts: []Options{jsonflags.StringifyBoolsAndStrings | 1}, + inBuf: `"\"foo\""`, + inVal: new(string), + want: addr("foo"), + }, { + name: jsontest.Name("Strings/StringifiedString/InvalidWhitespace"), + opts: []Options{jsonflags.StringifyBoolsAndStrings | 1}, + inBuf: `"\"foo\" "`, + inVal: new(string), + want: new(string), + wantErr: &SemanticError{action: "unmarshal", JSONKind: '"', GoType: stringType, Err: export.NewInvalidCharacterError(" ", "after string value", 0)}, + }, { + name: jsontest.Name("Strings/StringifiedString/InvalidString"), + opts: []Options{jsonflags.StringifyBoolsAndStrings | 1}, + inBuf: `""`, + inVal: new(string), + want: new(string), + wantErr: &SemanticError{action: "unmarshal", JSONKind: '"', GoType: stringType, Err: io.ErrUnexpectedEOF}, }, { name: jsontest.Name("Bytes/Null"), inBuf: `null`, @@ -5661,6 +5863,92 @@ func TestUnmarshal(t *testing.T) { inVal: new(structStringifiedAll), want: new(structStringifiedAll), wantErr: &SemanticError{action: "unmarshal", JSONKind: '"', GoType: int64Type, Err: fmt.Errorf(`cannot parse "" as signed integer: %w`, strconv.ErrSyntax)}, + }, { + name: jsontest.Name("Structs/LegacyStringified"), + opts: []Options{jsonflags.StringifyWithLegacySemantics | 1}, + inBuf: `{ + "Bool": "true", + "String": "\"hello\"", + "Bytes": "AQID", + "Int": "-64", + "Uint": "64", + "Float": "3.14159", + "Map": {"key": "value"}, + "StructScalars": { + "Bool": true, + "String": "hello", + "Bytes": "AQID", + "Int": -64, + "Uint": 64, + "Float": 3.14159 + }, + "StructMaps": { + "MapBool": {"": true}, + "MapString": {"": "hello"}, + "MapBytes": {"": "AQID"}, + "MapInt": {"": -64}, + "MapUint": {"": 64}, + "MapFloat": {"": 3.14159} + }, + "StructSlices": { + "SliceBool": [true], + "SliceString": ["hello"], + "SliceBytes": ["AQID"], + "SliceInt": [-64], + "SliceUint": [64], + "SliceFloat": [3.14159] + }, + "Slice": ["fizz", "buzz"], + "Array": ["goodbye"] +}`, + inVal: new(structStringifiedAll), + want: addr(structStringifiedAll{ + Bool: true, + String: "hello", + Bytes: []byte{1, 2, 3}, + Int: -64, + Uint: +64, + Float: 3.14159, + Map: map[string]string{"key": "value"}, + StructScalars: structScalars{ + Bool: true, + String: "hello", + Bytes: []byte{1, 2, 3}, + Int: -64, + Uint: +64, + Float: 3.14159, + }, + StructMaps: structMaps{ + MapBool: map[string]bool{"": true}, + MapString: map[string]string{"": "hello"}, + MapBytes: map[string][]byte{"": {1, 2, 3}}, + MapInt: map[string]int64{"": -64}, + MapUint: map[string]uint64{"": +64}, + MapFloat: map[string]float64{"": 3.14159}, + }, + StructSlices: structSlices{ + SliceBool: []bool{true}, + SliceString: []string{"hello"}, + SliceBytes: [][]byte{{1, 2, 3}}, + SliceInt: []int64{-64}, + SliceUint: []uint64{+64}, + SliceFloat: []float64{3.14159}, + }, + Slice: []string{"fizz", "buzz"}, + Array: [1]string{"goodbye"}, + }), + }, { + name: jsontest.Name("Structs/LegacyStringified/InvalidBool"), + opts: []Options{jsonflags.StringifyWithLegacySemantics | 1}, + inBuf: `{"Bool": true}`, + inVal: new(structStringifiedAll), + wantErr: &SemanticError{action: "unmarshal", JSONKind: 't', GoType: boolType}, + }, { + name: jsontest.Name("Structs/LegacyStringified/InvalidString"), + opts: []Options{jsonflags.StringifyWithLegacySemantics | 1}, + inBuf: `{"String": "string"}`, + inVal: new(structStringifiedAll), + wantErr: &SemanticError{action: "unmarshal", JSONKind: '"', GoType: stringType, Err: export.NewInvalidCharacterError("s", "at start of string (expecting '\"')", 0)}, }, { name: jsontest.Name("Structs/Format/Bytes"), inBuf: `{ diff --git a/internal/jsonflags/flags.go b/internal/jsonflags/flags.go index 119bccf..e15b7e4 100644 --- a/internal/jsonflags/flags.go +++ b/internal/jsonflags/flags.go @@ -129,6 +129,7 @@ const ( ReportLegacyErrorValues // marshal or unmarshal SkipUnaddressableMethods // marshal or unmarshal StringifyWithLegacySemantics // marshal or unmarshal + StringifyBoolsAndStrings // marshal or unmarshal; for internal use by jsonv2.makeStructArshaler UnmarshalAnyWithRawNumber // unmarshal; for internal use by jsonv1.Decoder.UseNumber UnmarshalArrayFromAnyLength // unmarshal diff --git a/v1/options.go b/v1/options.go index 743d580..57b8d09 100644 --- a/v1/options.go +++ b/v1/options.go @@ -38,6 +38,7 @@ type Options = jsonopts.Options // - [MatchCaseSensitiveDelimiter] // - [OmitEmptyWithLegacyDefinition] // - [RejectFloatOverflow] +// - [StringifyWithLegacySemantics] // - [UnmarshalArrayFromAnyLength] // - [jsonv2.Deterministic] // - [jsonv2.FormatNilSliceAsNull] @@ -147,6 +148,25 @@ func RejectFloatOverflow(v bool) Options { } } +// StringifyWithLegacySemantics specifies that the `string` tag option +// may stringify bools and string values. It only takes effect on fields +// where the top-level type is a bool, string, numeric kind, or a pointer to +// such a kind. Specifically, `string` will not stringify bool, string, +// or numeric kinds within a composite data type +// (e.g., array, slice, struct, map, or interface). +// +// This affects either marshaling or unmarshaling. +// The v1 default is true. +func StringifyWithLegacySemantics(v bool) Options { + // TODO: In v1, we would permit unmarshaling "null" (i.e., a quoted null) + // as if it were just null. We do not support this in v2. Should we? + if v { + return jsonflags.StringifyWithLegacySemantics | 1 + } else { + return jsonflags.StringifyWithLegacySemantics | 0 + } +} + // UnmarshalArrayFromAnyLength specifies that Go arrays can be unmarshaled // from input JSON arrays of any length. If the JSON array is too short, // then the remaining Go array elements are zeroed. If the JSON array