diff options
| author | Joe Tsai <joetsai@digital-static.net> | 2025-06-21 21:27:09 -0700 |
|---|---|---|
| committer | Gopher Robot <gobot@golang.org> | 2025-06-24 06:41:42 -0700 |
| commit | 2e9bb62bfed92ef24a6744fbdc3cf24eb672cd56 (patch) | |
| tree | d9c4a66c873fa306149de87d713d69f4555cc377 /src | |
| parent | ed7815726db4a0eb904d7cae2532cde48348d7ff (diff) | |
| download | go-2e9bb62bfed92ef24a6744fbdc3cf24eb672cd56.tar.xz | |
encoding/json/v2: reject unquoted dash as a JSON field name
In this blog:
https://blog.trailofbits.com/2025/06/17/unexpected-security-footguns-in-gos-parsers/
the concern was raised that whenever "-" is combined with other options,
the "-" is intepreted as as a name, rather than an ignored field,
which may go contrary to user expectation.
Static analysis demonstrates that there are ~2k instances of `json:"-,omitempty"
in the wild, where almost all of them intended for the field to be ignored.
To prevent this footgun, reject any tags that has "-," as a prefix
and warn the user to choose one of the reasonable alternatives.
The documentation of json/v2 already suggests `json:"'-'"`
as the recommended way to explicitly specify dash as the name.
See Example_fieldNames for example usages of the single-quoted literal.
Update the v1 json documentation to suggest the same thing.
Updates #71497
Change-Id: I7687b6eecdf82a5d894d057c78a4a90af4f5a6e4
Reviewed-on: https://go-review.googlesource.com/c/go/+/683175
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Johan Brandhorst-Satzkorn <johan.brandhorst@gmail.com>
Reviewed-by: Damien Neil <dneil@google.com>
Auto-Submit: Joseph Tsai <joetsai@digital-static.net>
Reviewed-by: Daniel Martà <mvdan@mvdan.cc>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Diffstat (limited to 'src')
| -rw-r--r-- | src/encoding/json/decode_test.go | 21 | ||||
| -rw-r--r-- | src/encoding/json/v2/fields.go | 8 | ||||
| -rw-r--r-- | src/encoding/json/v2/fields_test.go | 13 | ||||
| -rw-r--r-- | src/encoding/json/v2_decode_test.go | 21 | ||||
| -rw-r--r-- | src/encoding/json/v2_encode.go | 7 |
5 files changed, 68 insertions, 2 deletions
diff --git a/src/encoding/json/decode_test.go b/src/encoding/json/decode_test.go index 5bc3d3c856..473fd02833 100644 --- a/src/encoding/json/decode_test.go +++ b/src/encoding/json/decode_test.go @@ -1189,6 +1189,27 @@ var unmarshalTests = []struct { out: []int{1, 2, 0, 4, 5}, err: &UnmarshalTypeError{Value: "bool", Type: reflect.TypeFor[int](), Offset: 9}, }, + + { + CaseName: Name("DashComma"), + in: `{"-":"hello"}`, + ptr: new(struct { + F string `json:"-,"` + }), + out: struct { + F string `json:"-,"` + }{"hello"}, + }, + { + CaseName: Name("DashCommaOmitEmpty"), + in: `{"-":"hello"}`, + ptr: new(struct { + F string `json:"-,omitempty"` + }), + out: struct { + F string `json:"-,omitempty"` + }{"hello"}, + }, } func TestMarshal(t *testing.T) { diff --git a/src/encoding/json/v2/fields.go b/src/encoding/json/v2/fields.go index 9413189c08..4a02be7327 100644 --- a/src/encoding/json/v2/fields.go +++ b/src/encoding/json/v2/fields.go @@ -404,6 +404,7 @@ type fieldOptions struct { // the JSON member name and other features. func parseFieldOptions(sf reflect.StructField) (out fieldOptions, ignored bool, err error) { tag, hasTag := sf.Tag.Lookup("json") + tagOrig := tag // Check whether this field is explicitly ignored. if tag == "-" { @@ -453,6 +454,13 @@ func parseFieldOptions(sf reflect.StructField) (out fieldOptions, ignored bool, err = cmp.Or(err, fmt.Errorf("Go struct field %s has JSON object name %q with invalid UTF-8", sf.Name, name)) name = string([]rune(name)) // replace invalid UTF-8 with utf8.RuneError } + if name == "-" && tag[0] == '-' { + defer func() { // defer to let other errors take precedence + err = cmp.Or(err, fmt.Errorf("Go struct field %s has JSON object name %q; either "+ + "use `json:\"-\"` to ignore the field or "+ + "use `json:\"'-'%s` to specify %q as the name", sf.Name, out.name, strings.TrimPrefix(strconv.Quote(tagOrig), `"-`), name)) + }() + } if err2 == nil { out.hasName = true out.name = name diff --git a/src/encoding/json/v2/fields_test.go b/src/encoding/json/v2/fields_test.go index 1c36f80905..ae58182f29 100644 --- a/src/encoding/json/v2/fields_test.go +++ b/src/encoding/json/v2/fields_test.go @@ -503,6 +503,19 @@ func TestParseTagOptions(t *testing.T) { wantOpts: fieldOptions{hasName: true, name: "-", quotedName: `"-"`}, wantErr: errors.New("Go struct field V has malformed `json` tag: invalid trailing ',' character"), }, { + name: jsontest.Name("DashCommaOmitEmpty"), + in: struct { + V int `json:"-,omitempty"` + }{}, + wantOpts: fieldOptions{hasName: true, name: "-", quotedName: `"-"`, omitempty: true}, + wantErr: errors.New("Go struct field V has JSON object name \"-\"; either use `json:\"-\"` to ignore the field or use `json:\"'-',omitempty\"` to specify \"-\" as the name"), + }, { + name: jsontest.Name("QuotedDashCommaOmitEmpty"), + in: struct { + V int `json:"'-',omitempty"` + }{}, + wantOpts: fieldOptions{hasName: true, name: "-", quotedName: `"-"`, omitempty: true}, + }, { name: jsontest.Name("QuotedDashName"), in: struct { V int `json:"'-'"` diff --git a/src/encoding/json/v2_decode_test.go b/src/encoding/json/v2_decode_test.go index fe814a3cfd..3ab20e2b5d 100644 --- a/src/encoding/json/v2_decode_test.go +++ b/src/encoding/json/v2_decode_test.go @@ -1195,6 +1195,27 @@ var unmarshalTests = []struct { out: []int{1, 2, 0, 4, 5}, err: &UnmarshalTypeError{Value: "bool", Type: reflect.TypeFor[int](), Field: "2", Offset: len64(`[1,2,`)}, }, + + { + CaseName: Name("DashComma"), + in: `{"-":"hello"}`, + ptr: new(struct { + F string `json:"-,"` + }), + out: struct { + F string `json:"-,"` + }{"hello"}, + }, + { + CaseName: Name("DashCommaOmitEmpty"), + in: `{"-":"hello"}`, + ptr: new(struct { + F string `json:"-,omitempty"` + }), + out: struct { + F string `json:"-,omitempty"` + }{"hello"}, + }, } func TestMarshal(t *testing.T) { diff --git a/src/encoding/json/v2_encode.go b/src/encoding/json/v2_encode.go index c8f35d4281..cbb167dbd0 100644 --- a/src/encoding/json/v2_encode.go +++ b/src/encoding/json/v2_encode.go @@ -68,7 +68,10 @@ import ( // slice, map, or string of length zero. // // As a special case, if the field tag is "-", the field is always omitted. -// Note that a field with name "-" can still be generated using the tag "-,". +// JSON names containing commas or quotes, or names identical to "" or "-", +// can be specified using a single-quoted string literal, where the syntax +// is identical to the Go grammar for a double-quoted string literal, +// but instead uses single quotes as the delimiters. // // Examples of struct field tags and their meanings: // @@ -89,7 +92,7 @@ import ( // Field int `json:"-"` // // // Field appears in JSON as key "-". -// Field int `json:"-,"` +// Field int `json:"'-'"` // // The "omitzero" option specifies that the field should be omitted // from the encoding if the field has a zero value, according to rules: |
