Skip to content

Commit 6fd8e88

Browse files
committed
encoding/json/v2: restrict presence of default options
Originally, DefaultOptionsV1 and DefaultOptionsV2 represented the full set of all options with specific ones set to true or false. However, there are certain options such as WithIndent or WithMarshalers that are neither v1 or v2 specific. At some point we removed whitespace related options from the set: go-json-experiment/json#26 This avoids DefaultOptionsV1 or DefaultOptionsV2 from affecting any previously set whitespace. However, why are whitespace options special and thus excluded from the set? What about Marshalers? As a more principaled way to address this, we restrict DefaultOptionsV1 and DefaultOptionsV2 to only be the options where the default setting changes between v1 and v2. All other options are unpopulated. This avoids a panic with GetOption(DefaultOptionsV2, WithMarshalers) since DefaultOptionsV2 previously had the presence bit for Marshalers set to true, but had no actual value. Now, the presence bit is set to false, so the value is not consulted. Fixes #75149 Change-Id: I30b45abd35404578b4135cc3bad1a1a2993cb0cf Reviewed-on: https://go-review.googlesource.com/c/go/+/710878 Reviewed-by: Johan Brandhorst-Satzkorn <johan.brandhorst@gmail.com> Reviewed-by: Michael Pratt <mpratt@google.com> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Damien Neil <dneil@google.com>
1 parent 1abc6b0 commit 6fd8e88

File tree

4 files changed

+10
-10
lines changed

4 files changed

+10
-10
lines changed

src/encoding/json/internal/jsonopts/options.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -48,16 +48,16 @@ type ArshalValues struct {
4848
// DefaultOptionsV2 is the set of all options that define default v2 behavior.
4949
var DefaultOptionsV2 = Struct{
5050
Flags: jsonflags.Flags{
51-
Presence: uint64(jsonflags.AllFlags & ^jsonflags.WhitespaceFlags),
52-
Values: uint64(0),
51+
Presence: uint64(jsonflags.DefaultV1Flags),
52+
Values: uint64(0), // all flags in DefaultV1Flags are false
5353
},
5454
}
5555

5656
// DefaultOptionsV1 is the set of all options that define default v1 behavior.
5757
var DefaultOptionsV1 = Struct{
5858
Flags: jsonflags.Flags{
59-
Presence: uint64(jsonflags.AllFlags & ^jsonflags.WhitespaceFlags),
60-
Values: uint64(jsonflags.DefaultV1Flags),
59+
Presence: uint64(jsonflags.DefaultV1Flags),
60+
Values: uint64(jsonflags.DefaultV1Flags), // all flags in DefaultV1Flags are true
6161
},
6262
}
6363

src/encoding/json/internal/jsonopts/options_test.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -200,6 +200,9 @@ func TestGet(t *testing.T) {
200200
if v, ok := json.GetOption(opts, json.WithUnmarshalers); v != nil || ok {
201201
t.Errorf(`GetOption(..., WithUnmarshalers) = (%v, %v), want (nil, false)`, v, ok)
202202
}
203+
if v, ok := json.GetOption(json.DefaultOptionsV2(), json.WithMarshalers); v != nil || ok {
204+
t.Errorf(`GetOption(..., WithMarshalers) = (%v, %v), want (nil, false)`, v, ok)
205+
}
203206
}
204207

205208
var sink struct {

src/encoding/json/v2/options.go

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -97,9 +97,8 @@ func GetOption[T any](opts Options, setter func(T) Options) (T, bool) {
9797
}
9898

9999
// DefaultOptionsV2 is the full set of all options that define v2 semantics.
100-
// It is equivalent to all options under [Options], [encoding/json.Options],
101-
// and [encoding/json/jsontext.Options] being set to false or the zero value,
102-
// except for the options related to whitespace formatting.
100+
// It is equivalent to the set of options in [encoding/json.DefaultOptionsV1]
101+
// all being set to false. All other options are not present.
103102
func DefaultOptionsV2() Options {
104103
return &jsonopts.DefaultOptionsV2
105104
}

src/encoding/json/v2_options.go

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -227,9 +227,7 @@ type Options = jsonopts.Options
227227
// - [jsontext.EscapeForJS]
228228
// - [jsontext.PreserveRawStrings]
229229
//
230-
// All other boolean options are set to false.
231-
// All non-boolean options are set to the zero value,
232-
// except for [jsontext.WithIndent], which defaults to "\t".
230+
// All other options are not present.
233231
//
234232
// The [Marshal] and [Unmarshal] functions in this package are
235233
// semantically identical to calling the v2 equivalents with this option:

0 commit comments

Comments
 (0)