Map serialization using reflect.Value.MapIndex cannot retrieve
map keys that contain a NaN, resulting in a panic.
Switch the implementation to use the reflect.Value.MapRange method
instead, which iterates over all map entries regardless of whether
they are directly retrievable.
Note that according to RFC 8259, section 4, a JSON object should
have unique names, but does not forbid the occurrence of duplicate names.
Fixes#43207
Change-Id: If4bc55229b1f64b8ca4b0fed37549725efdace39
Reviewed-on: https://go-review.googlesource.com/c/go/+/278632
Trust: Meng Zhuo <mzh@golangcn.org>
Trust: Joe Tsai <thebrokentoaster@gmail.com>
Trust: Bryan C. Mills <bcmills@google.com>
Run-TryBot: Meng Zhuo <mzh@golangcn.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
Now reports an error if cyclic maps and slices are to be encoded
instead of an infinite recursion. This case wasn't handled in CL 187920.
Fixes#40745.
Change-Id: Ia34b014ecbb71fd2663bb065ba5355a307dbcc15
GitHub-Last-Rev: 6f874944f4065b5237babbb0fdce14c1c74a3c97
GitHub-Pull-Request: golang/go#40756
Reviewed-on: https://go-review.googlesource.com/c/go/+/248358
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
Trust: Daniel Martí <mvdan@mvdan.cc>
Trust: Bryan C. Mills <bcmills@google.com>
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Go Bot <gobot@golang.org>
This reverts CL 254537.
Reason for revert: Reason for revert: The recommended way to check for a type of error is errors.As. API changes should also start with a proposal.
Change-Id: I07c37428575e99c80b17525833a61831d10963bb
Reviewed-on: https://go-review.googlesource.com/c/go/+/254857
Trust: Damien Neil <dneil@google.com>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
Allows users to check:
errors.Is(err, &UnmarshalTypeError{})
errors.Is(err, &UnmarshalFieldError{})
errors.Is(err, &InvalidUnmarshalError{})
errors.Is(err, &UnsupportedValueError{})
errors.Is(err, &MarshalerError{})
which is the recommended way of checking for kinds of errors.
SyntaxError.Is was implemented in CL 253037.
As and Unwrap relevant methods will be added in future CLs.
Change-Id: I1f8a503b8fdc0f3afdfe9669a91f3af8d960e028
GitHub-Last-Rev: 930cda5384c987a0b31f277ba3b4ab690ea74ac3
GitHub-Pull-Request: golang/go#41360
Reviewed-on: https://go-review.googlesource.com/c/go/+/254537
Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
Trust: Emmanuel Odeke <emm.odeke@gmail.com>
golang.org/cl/193604 fixed one bug when one encodes a string with the
",string" option: if SetEscapeHTML(false) is used, we should not be
using HTML escaping for the inner string encoding. The CL correctly
fixed that.
The CL also tried to speed up this edge case. By avoiding an entire new
call to Marshal, the new Issue34127 benchmark reduced its time/op by
45%, and lowered the allocs/op from 3 to 2.
However, that last optimization wasn't correct:
Since Go 1.2 every string can be marshaled to JSON without error
even if it contains invalid UTF-8 byte sequences. Therefore
there is no need to use Marshal again for the only reason of
enclosing the string in double quotes.
JSON string encoding isn't just about adding quotes and taking care of
invalid UTF-8. We also need to escape some characters, like tabs and
newlines.
The new code failed to do that. The bug resulted in the added test case
failing to roundtrip properly; before our fix here, we'd see an error:
invalid use of ,string struct tag, trying to unmarshal "\"\b\f\n\r\t\"\\\"" into string
If you pay close attention, you'll notice that the special characters
like tab and newline are only encoded once, not twice. When decoding
with the ",string" option, the outer string decode works, but the inner
string decode fails, as we are now decoding a JSON string with unescaped
special characters.
The fix we apply here isn't to go back to Marshal, as that would
re-introduce the bug with SetEscapeHTML(false). Instead, we can use a
new encode state from the pool - it results in minimal performance
impact, and even reduces allocs/op further. The performance impact seems
fair, given that we need to check the entire string for characters that
need to be escaped.
name old time/op new time/op delta
Issue34127-8 89.7ns ± 2% 100.8ns ± 1% +12.27% (p=0.000 n=8+8)
name old alloc/op new alloc/op delta
Issue34127-8 40.0B ± 0% 32.0B ± 0% -20.00% (p=0.000 n=8+8)
name old allocs/op new allocs/op delta
Issue34127-8 2.00 ± 0% 1.00 ± 0% -50.00% (p=0.000 n=8+8)
Instead of adding another standalone test, we convert an existing
"string tag" test to be table-based, and add another test case there.
One test case from the original CL also had to be amended, due to the
same problem - when escaping '<' due to SetEscapeHTML(true), we need to
end up with double escaping, since we're using ",string".
Fixes#38173.
Change-Id: I2b0df9e4f1d3452fff74fe910e189c930dde4b5b
Reviewed-on: https://go-review.googlesource.com/c/go/+/226498
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
Otherwise we'd panic with a stack overflow.
Most programs are in control of the data being encoded and can ensure
there are no cycles, but sometimes it's not that simple. For example,
running a user's html template with script tags can easily result in
crashes if the user can find a pointer cycle.
Adding the checks via a map to every ptrEncoder.encode call slowed down
the benchmarks below by a noticeable 13%. Instead, only start doing the
relatively expensive pointer cycle checks if we're many levels of
pointers deep in an encode state.
A threshold of 1000 is small enough to capture pointer cycles before
they're a problem (the goroutine stack limit is currently 1GB, and I
needed close to a million levels to reach it). Yet it's large enough
that reasonable uses of the json encoder only see a tiny 1% slow-down
due to the added ptrLevel field and check.
name old time/op new time/op delta
CodeEncoder-8 2.34ms ± 1% 2.37ms ± 0% +1.05% (p=0.000 n=10+10)
CodeMarshal-8 2.42ms ± 1% 2.44ms ± 0% +1.10% (p=0.000 n=10+10)
name old speed new speed delta
CodeEncoder-8 829MB/s ± 1% 820MB/s ± 0% -1.04% (p=0.000 n=10+10)
CodeMarshal-8 803MB/s ± 1% 795MB/s ± 0% -1.09% (p=0.000 n=10+10)
name old alloc/op new alloc/op delta
CodeEncoder-8 43.1kB ± 8% 42.5kB ±10% ~ (p=0.989 n=10+10)
CodeMarshal-8 1.99MB ± 0% 1.99MB ± 0% ~ (p=0.254 n=9+6)
name old allocs/op new allocs/op delta
CodeEncoder-8 0.00 0.00 ~ (all equal)
CodeMarshal-8 1.00 ± 0% 1.00 ± 0% ~ (all equal)
Finally, add a few tests to ensure that the code handles the edge cases
properly.
Fixes#10769.
Change-Id: I73d48e0cf6ea140127ea031f2dbae6e6a55e58b8
Reviewed-on: https://go-review.googlesource.com/c/go/+/187920
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Bjørn Erik Pedersen <bjorn.erik.pedersen@gmail.com>
Reviewed-by: Andrew Bonventre <andybons@golang.org>
1. Change mapencode.encode to use fmt.Error rather than MarshalerError.
MarshalerError refer to MarshalJSON, but mapencode.encode does not use that.
2. Add sourceFunc field to MarshalerError to record the name of the function
that creates the error, so that the Error method can report it correctly.
Fixes#29753
Change-Id: I186c2fac8470ae2f9e300501de3730face642230
Reviewed-on: https://go-review.googlesource.com/c/go/+/184119
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Add quotes when marshaling a json.Number with the string option
set via a struct tag. This ensures that the resulting json
can be unmarshaled into the source struct without error.
Fixes#34268
Change-Id: Ide167d9dec77019554870b5957b37dc258119d81
GitHub-Last-Rev: dde81b71208be01c253bb87dbb6f81ac6e0785be
GitHub-Pull-Request: golang/go#34269
Reviewed-on: https://go-review.googlesource.com/c/go/+/195043
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This change adds a a check in the encodeWithString.resolve method
to ensure that a reflect.Value with kind Ptr is not nil before
the type assertion to TextMarshaler.
If the value is nil, the method returns a nil error, and the map key
encodes to an empty string.
Fixes#33675
Change-Id: I0a04cf690ae67006f6a9c5f8cbb4cc99d236bca8
GitHub-Last-Rev: 6c987c90846f854e21814dbfb3a073605ec8a94c
GitHub-Pull-Request: golang/go#33700
Reviewed-on: https://go-review.googlesource.com/c/go/+/190697
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
Shorten some of the longest tests that run during all.bash.
Removes 7r 50u 21s from all.bash.
After this change, all.bash is under 5 minutes again on my laptop.
For #26473.
Change-Id: Ie0460aa935808d65460408feaed210fbaa1d5d79
Reviewed-on: https://go-review.googlesource.com/c/go/+/177559
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
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>
