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>

diff --git a/src/encoding/json/decode_test.go b/src/encoding/json/decode_test.go
index 5bc3d3c856413909244503ebc67279d52516b69c..473fd02833016d0cf11880bc66857f4abc06f91e 100644 (file)
--- 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 9413189c0850da49933cfaba3003e6e263c2fb1d..4a02be7327a04b9ffae1ab3e8e565b9318d9c7a7 100644 (file)
--- 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 1c36f80905246fd2ec23aa44146e4786f2f8a2d8..ae58182f298ab4d05b7758a3533d7bfbb4d74033 100644 (file)
--- a/src/encoding/json/v2/fields_test.go
+++ b/src/encoding/json/v2/fields_test.go
@@ -502,6 +502,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 {

diff --git a/src/encoding/json/v2_decode_test.go b/src/encoding/json/v2_decode_test.go
index fe814a3cfd52c0428393a93f8af0bce1581d3378..3ab20e2b5d06032074d967d281d41a513c58e121 100644 (file)
--- 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 c8f35d4281c751434d57b0d91f87afc26efdd4e5..cbb167dbd0df4091e3979ec0c46a6e04b9810109 100644 (file)
--- 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: