Given the following types:
type S2 struct{ Field string }
type S struct{ *S2 }
Marshalling a value of type T1 should result in "{}", as there's no way
to access any value of T2.Field. This is how Go 1.10 and earlier
versions behave.
However, in the recent refactor golang.org/cl/125417 I broke this logic.
When the encoder found an anonymous struct pointer field that was nil,
it no longer skipped the embedded fields underneath it. This can be seen
in the added test:
--- FAIL: TestAnonymousFields/EmbeddedFieldBehindNilPointer (0.00s)
encode_test.go:430: Marshal() = "{\"Field\":\"\\u003c*json.S2 Value\\u003e\"}", want "{}"
The human error was a misplaced label, meaning we weren't actually
skipping the right loop iteration. Fix that.
Change-Id: Iba8a4a77d358dac73dcba4018498fe4f81afa263
Reviewed-on: https://go-review.googlesource.com/131376
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Struct field names are static, so we can run HTMLEscape on them when
building each struct type encoder. Then, when running the struct
encoder, we can select either the original or the escaped field name to
write directly.
When the encoder is not escaping HTML, using the original string works
because neither Go struct field names nor JSON tags allow any characters
that would need to be escaped, like '"', '\\', or '\n'.
When the encoder is escaping HTML, the only difference is that '<', '>',
and '&' are allowed via JSON struct field tags, hence why we use
HTMLEscape to properly escape them.
All of the above lets us encode field names with a simple if/else and
WriteString calls, which are considerably simpler and faster than
encoding an arbitrary string.
While at it, also include the quotes and colon in these strings, to
avoid three WriteByte calls in the loop hot path.
Also added a few tests, to ensure that the behavior in these edge cases
is not broken. The output of the tests is the same if this optimization
is reverted.
name old time/op new time/op delta
CodeEncoder-4 7.12ms ± 0% 6.14ms ± 0% -13.85% (p=0.004 n=6+5)
name old speed new speed delta
CodeEncoder-4 272MB/s ± 0% 316MB/s ± 0% +16.08% (p=0.004 n=6+5)
name old alloc/op new alloc/op delta
CodeEncoder-4 91.9kB ± 0% 93.2kB ± 0% +1.43% (p=0.002 n=6+6)
name old allocs/op new allocs/op delta
CodeEncoder-4 0.00 0.00 ~ (all equal)
Updates #5683.
Change-Id: I6f6a340d0de4670799ce38cf95b2092822d2e3ef
Reviewed-on: https://go-review.googlesource.com/122460
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Rather than only ignoring runtime.Error panics, which are a very
narrow set of possible panic values, switch it such that the json
package only captures panic values that have been properly wrapped
in a jsonError struct. This ensures that only intentional panics
originating from the json package are captured.
Fixes#23012
Change-Id: I5e85200259edd2abb1b0512ce6cc288849151a6d
Reviewed-on: https://go-review.googlesource.com/94019
Run-TryBot: Joe Tsai <thebrokentoaster@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
This CL reverts CL 76851 and takes a different approach to #21357.
The changes in encode.go and encode_test.go are reverts that
rolls back the changed behavior in CL 76851 where
embedded pointers to unexported struct types were
unilaterally ignored in both marshal and unmarshal.
Instead, these fields are handled as before with the exception that
it returns an error when Unmarshal is unable to set an unexported field.
The behavior of Marshal is now unchanged with regards to #21357.
This policy maintains the greatest degree of backwards compatibility
and avoids silently discarding data the user may have expected to be present.
Fixes#21357
Change-Id: I7dc753280c99f786ac51acf7e6c0246618c8b2b1
Reviewed-on: https://go-review.googlesource.com/82135
Run-TryBot: Joe Tsai <thebrokentoaster@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
CL 60410 fixes a bug in reflect that allows assignments to an embedded
field of a pointer to an unexported struct type.
This breaks the json package because unmarshal is now unable to assign
a newly allocated struct to such fields.
In order to be consistent in the behavior for marshal and unmarshal,
this CL changes both marshal and unmarshal to always ignore
embedded pointers to unexported structs.
Fixes#21357
Change-Id: If62ea11155555e61115ebb9cfa5305caf101bde5
Reviewed-on: https://go-review.googlesource.com/76851
Run-TryBot: Joe Tsai <thebrokentoaster@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
The go repository contains a mix of github.com/golang/go/issues/xxxxx
and golang.org/issues/xxxxx URLs for references to issues in the issue
tracker. We should use one for consistency, and golang.org is preferred
in case the project moves the issue tracker in the future.
This reasoning is taken from a comment Sam Whited left on a CL I
recently opened: https://go-review.googlesource.com/c/go/+/73890.
In that CL I referenced an issue using its github.com URL, because other
tests in the file I was changing contained references to issues using
their github.com URL. Sam Whited left a comment on the CL stating I
should change it to the golang.org URL.
If new code is intended to reference issues via golang.org and not
github.com, existing code should be updated so that precedence exists
for contributors who are looking at the existing code as a guide for the
code they should write.
Change-Id: I3b9053fe38a1c56fc101a8b7fd7b8f310ba29724
Reviewed-on: https://go-review.googlesource.com/75673
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
https://golang.org/cl/33773 fixes the JSON marshaler to avoid serializing
embedded fields on unexported types of non-struct types. However, Go allows
embedding pointer to types, so the check for whether the field is a non-struct
type must first dereference the pointer to get at the underlying type.
Furthermore, due to a edge-case in the behavior of StructField.PkgPath not
being a reliable indicator of whether the field is unexported (see #21122),
we use our own logic to determine whether the field is exported or not.
The logic in this CL may be simplified depending on what happens in #21122.
Fixes#21121
Updates #21122
Change-Id: I8dfd1cdfac8a87950df294a566fb96dfd04fd749
Reviewed-on: https://go-review.googlesource.com/50711
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Marshal must process unexported embedded fields of struct type,
looking for exported fields in those structs. However, it must
not process unexported embedded fields of non-struct type.
For example, consider:
type t1 struct {
X int
}
type t2 int
type T struct {
t1
t2
}
When considering T, Marshal must process t1 to find t1.X.
Marshal must not process t2, but it was. Fix that.
Fixes#18009
Change-Id: I62ba0b65ba30fd927990e101a26405a9998787a3
Reviewed-on: https://go-review.googlesource.com/33773
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
The tree is inconsistent about single l vs double l in those
words in documentation, test messages, and one error value text.
$ git grep -E '[Mm]arshall(|s|er|ers|ed|ing)' | wc -l
42
$ git grep -E '[Mm]arshal(|s|er|ers|ed|ing)' | wc -l
1694
Make it consistently a single l, per earlier decisions. This means
contributors won't be confused by misleading precedence, and it helps
consistency.
Change the spelling in one error value text in newRawAttributes of
crypto/x509 package to be consistent.
This change was generated with:
perl -i -npe 's,([Mm]arshal)l(|s|er|ers|ed|ing),$1$2,' $(git grep -l -E '[Mm]arshall' | grep -v AUTHORS | grep -v CONTRIBUTORS)
Updates #12431.
Follows https://golang.org/cl/14150.
Change-Id: I85d28a2d7692862ccb02d6a09f5d18538b6049a2
Reviewed-on: https://go-review.googlesource.com/33017
Run-TryBot: Minux Ma <minux@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
I used the slowtests.go tool as described in
https://golang.org/cl/32684 on packages that stood out.
go test -short std drops from ~56 to ~52 seconds.
This isn't a huge win, but it was mostly an exercise.
Updates #17751
Change-Id: I9f3402e36a038d71e662d06ce2c1d52f6c4b674d
Reviewed-on: https://go-review.googlesource.com/32751
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
This CL expands upon a change made in (http://golang.org/cl/21811)
to ensure that a nil RawMessage gets serialized as "null" instead of
being a nil slice.
The added check only triggers when the RawMessage is nil. We do not
handle the case when the RawMessage is non-nil, but empty.
Fixes#17704
Updates #14493
Change-Id: I0fbebcdd81f7466c5b78c94953afc897f162ceb4
Reviewed-on: https://go-review.googlesource.com/32472
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Change float32/float64 formatting to use non-exponential form
for a slightly wider range, to more closely match ES6 JSON.stringify
and other JSON generators.
Most notably:
1e20 now formats as 100000000000000000000 (previously 1e+20)
1e-6 now formats as 0.000001 (previously 1e-06)
1e-7 now formats as 1e-7 (previously 1e-07)
This also brings the int64 and float64 formatting in line with each other,
for all shared representable values. For example both int64(1234567)
and float64(1234567) now format as "1234567", where before the
float64 formatted as "1.234567e+06".
The only variation now compared to ES6 JSON.stringify is that
Go continues to encode negative zero as "-0", not "0", so that
the value continues to be preserved during JSON round trips.
Fixes#6384.
Fixes#14135.
Change-Id: Ib0e0e009cd9181d75edc0424a28fe776bcc5bbf8
Reviewed-on: https://go-review.googlesource.com/30371
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
This provides a way to disable the escaping of <, >, and & in JSON
strings.
Fixes#14749.
Change-Id: I1afeb0244455fc8b06c6cce920444532f229555b
Reviewed-on: https://go-review.googlesource.com/21796
Run-TryBot: Caleb Spare <cespare@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
This CL allows JSON-encoding & -decoding maps whose keys are types that
implement encoding.TextMarshaler / TextUnmarshaler.
During encode, the map keys are marshaled upfront so that they can be
sorted.
Fixes#12146
Change-Id: I43809750a7ad82a3603662f095c7baf75fd172da
Reviewed-on: https://go-review.googlesource.com/20356
Run-TryBot: Caleb Spare <cespare@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
json.Number is a special case which didn't have any checks and could result in invalid JSON.
Fixes#10281
Change-Id: Ie3e726e4d6bf6a6aba535d36f6107013ceac913a
Reviewed-on: https://go-review.googlesource.com/12250
Reviewed-by: Russ Cox <rsc@golang.org>
As correctly mentioned in #11883, encodeState.string and
encodeState.stringBytes never return an error.
This CL removes the error from the function signatures and somewhat
simplifies call sites.
Fixes#11883
Change-Id: I1d1853d09631c545b68b5eea86ff7daa2e0ca10b
Reviewed-on: https://go-review.googlesource.com/15836
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
