Many uses of Index/IndexByte/IndexRune/Split/SplitN
can be written more clearly using the new Cut functions.
Do that. Also rewrite to other functions if that's clearer.
For #46336.
Change-Id: I68d024716ace41a57a8bf74455c62279bde0f448
Reviewed-on: https://go-review.googlesource.com/c/go/+/351711
Trust: Russ Cox <rsc@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Updates golang/go#37595
Change-Id: I7568e7416d5504e9dc67061c79f66e3a0d597dee
Reviewed-on: https://go-review.googlesource.com/c/go/+/351470
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Trust: David Chase <drchase@google.com>
xml names can't have any of '[],' in them, which might appear in
generic type names. Truncate at the first '[' so the names are still valid.
Fixes#48318
Change-Id: I110ff4269f763089467e7cf84b0f0c5075fb44b7
Reviewed-on: https://go-review.googlesource.com/c/go/+/349349
Trust: Keith Randall <khr@golang.org>
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
In CL 345572, we used the reflect.Value.SetLen method to avoid
extra memory allocation for reflect.Value.Slice.
This also applies to function decodeSlice
name old time/op new time/op delta
DecodeStringsSlice-12 96.5µs ±12% 63.0µs ± 8% -34.68% (p=0.000 n=9+10)
name old alloc/op new alloc/op delta
DecodeStringsSlice-12 89.3kB ± 0% 65.3kB ± 0% -26.89% (p=0.000 n=10+10)
name old allocs/op new allocs/op delta
DecodeStringsSlice-12 3.18k ± 0% 2.18k ± 0% -31.47% (p=0.000 n=10+10)
Change-Id: Ifdb43716cc90a265962dec022704a5571f447fd8
Reviewed-on: https://go-review.googlesource.com/c/go/+/347533
Reviewed-by: Joe Tsai <joetsai@digital-static.net>
Reviewed-by: Rob Pike <r@golang.org>
Trust: Joe Tsai <joetsai@digital-static.net>
Run-TryBot: Joe Tsai <joetsai@digital-static.net>
TryBot-Result: Go Bot <gobot@golang.org>
golang.org/cl/33572 added a map iterator.
use the reflect.Value.MapRange to fix map keys that contain a NaN
Fixes#24075
Change-Id: I0214d6f26c2041797703e48eac16404f189d6982
GitHub-Last-Rev: 5c01e117f4
GitHub-Pull-Request: golang/go#47476
Reviewed-on: https://go-review.googlesource.com/c/go/+/338609
Trust: Bryan C. Mills <bcmills@google.com>
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Rob Pike <r@golang.org>
The reflect.Value.Slice method unfortunately allocates every time
since it needs to place the slice header on the heap.
This is silly since gob immediately stores the result back into slice.
Instead, use the reflect.Value.SetLen method.
DecodeBytesSlice 75.0µs ± 2% 35.2µs ± 6% -53.02%
Change-Id: I3ca0529d01bf978f2b76e215f52d369f458951ef
Reviewed-on: https://go-review.googlesource.com/c/go/+/345572
Trust: Joe Tsai <joetsai@digital-static.net>
Run-TryBot: Joe Tsai <joetsai@digital-static.net>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Rob Pike <r@golang.org>
This enables a consumer of a CSV to find out the position
of a CSV field without implementing an intermediate buffer.
This is useful to produce good higher level error messages when
the CSV syntax is OK but the field values don't match expectations.
This also changes the existing semantics of the `ParseError.Column`
field to bring it in line with precedent elsewhere in the Go
standard library (notably go/token.Position) - the column is
now 1-based and indicates a byte count rather than a rune count,
and the error position reporting at the end of a last line without
a newline is now fixed.
This change has some impact on performance:
```
name old time/op new time/op delta
Read-8 2.14µs ± 0% 2.15µs ± 0% ~ (p=0.056 n=5+5)
ReadWithFieldsPerRecord-8 2.15µs ± 2% 2.15µs ± 1% ~ (p=0.151 n=5+5)
ReadWithoutFieldsPerRecord-8 2.15µs ± 0% 2.15µs ± 0% +0.37% (p=0.024 n=5+5)
ReadLargeFields-8 3.55µs ± 2% 3.59µs ± 0% ~ (p=0.206 n=5+5)
ReadReuseRecord-8 1.18µs ± 1% 1.22µs ± 1% +2.93% (p=0.008 n=5+5)
ReadReuseRecordWithFieldsPerRecord-8 1.18µs ± 0% 1.21µs ± 0% +2.54% (p=0.008 n=5+5)
ReadReuseRecordWithoutFieldsPerRecord-8 1.18µs ± 0% 1.22µs ± 1% +3.66% (p=0.008 n=5+5)
ReadReuseRecordLargeFields-8 2.53µs ± 1% 2.57µs ± 1% +1.70% (p=0.008 n=5+5)
Write-8 1.02µs ± 1% 1.01µs ± 0% -1.18% (p=0.016 n=5+4)
```
Fixes#44221.
Change-Id: Id37c50fc396024eef406c5bad45380ecd414f5ea
Reviewed-on: https://go-review.googlesource.com/c/go/+/291290
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Trust: Paul Jolly <paul@myitcv.org.uk>
A Directive (like <!ENTITY xxx []>) can't have other nodes nested inside
it (in our data structure representation), so there is no way to
preserve comments. The previous behavior was to just elide them, which
however might change the semantic meaning of the surrounding markup.
Instead, replace them with a space which hopefully has the same semantic
effect of the comment.
Directives are not actually a node type in the XML spec, which instead
specifies each of them separately (<!ENTITY, <!DOCTYPE, etc.), each with
its own grammar. The rules for where and when the comments are allowed
are not straightforward, and can't be implemented without implementing
custom logic for each of the directives.
Simply preserving the comments in the body of the directive would be
problematic, as there can be unmatched quotes inside the comment.
Whether those quotes are considered meaningful semantically or not,
other parsers might disagree and interpret the output differently.
This issue was reported by Juho Nurminen of Mattermost as it leads to
round-trip mismatches. See #43168. It's not being fixed in a security
release because round-trip stability is not a currently supported
security property of encoding/xml, and we don't believe these fixes
would be sufficient to reliably guarantee it in the future.
Fixes CVE-2020-29510
Updates #43168
Change-Id: Icd86c75beff3e1e0689543efebdad10ed5178ce3
Reviewed-on: https://go-review.googlesource.com/c/go/+/277893
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Filippo Valsorda <filippo@golang.org>
Reviewed-by: Katie Hockman <katie@golang.org>
Before this change, <:name> would parse as <name>, which could cause
issues in applications that rely on the parse-encode cycle to
round-trip. Similarly, <x name:=""> would parse as expected but then
have the attribute dropped when serializing because its name was empty.
Finally, <a🅱️c> would parse and get serialized incorrectly. All these
values are invalid XML, but to minimize the impact of this change, we
parse them whole into Name.Local.
This issue was reported by Juho Nurminen of Mattermost as it leads to
round-trip mismatches. See #43168. It's not being fixed in a security
release because round-trip stability is not a currently supported
security property of encoding/xml, and we don't believe these fixes
would be sufficient to reliably guarantee it in the future.
Fixes CVE-2020-29509
Fixes CVE-2020-29511
Updates #43168
Change-Id: I68321c4d867305046f664347192948a889af3c7f
Reviewed-on: https://go-review.googlesource.com/c/go/+/277892
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Filippo Valsorda <filippo@golang.org>
Reviewed-by: Katie Hockman <katie@golang.org>
Prior to this CL, pasting the example from the website causes a
compilation error for some programs because it was shadowing the
"json" package.
Change-Id: I39b68a66ca99468547f2027a7655cf1387b61e95
Reviewed-on: https://go-review.googlesource.com/c/go/+/301492
Reviewed-by: Joe Tsai <joetsai@google.com>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Trust: Joe Tsai <joetsai@google.com>
Run-TryBot: Joe Tsai <joetsai@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Previously re-using a decoder with a new stream resulted in a confusing
"extra data in buffer" error message.
Change-Id: Ia4c4c3a2d4b63c59e37e53faa61a500d5ff6e5f1
Reviewed-on: https://go-review.googlesource.com/c/go/+/297949
Reviewed-by: Rob Pike <r@golang.org>
Run-TryBot: Rob Pike <r@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Emmanuel Odeke <emmanuel@orijtech.com>
This change properly handles a TokenReader which
returns an EOF in the middle of an open XML
element.
Thanks to Sam Whited for reporting this.
Fixes CVE-2021-27918
Fixes#44913
Change-Id: Id02a3f3def4a1b415fa2d9a8e3b373eb6cb0f433
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/1004594
Reviewed-by: Russ Cox <rsc@google.com>
Reviewed-by: Roland Shoemaker <bracewell@google.com>
Reviewed-by: Filippo Valsorda <valsorda@google.com>
Reviewed-on: https://go-review.googlesource.com/c/go/+/300391
Trust: Katie Hockman <katie@golang.org>
Run-TryBot: Katie Hockman <katie@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Alexander Rakoczy <alex@golang.org>
Reviewed-by: Filippo Valsorda <filippo@golang.org>
Limits the number of bytes that can be consumed by Uvarint
to MaxVarintLen64 (10) to avoid wasted computations.
With this change, if Uvarint reads more than MaxVarintLen64
bytes, it'll return the erroring byte count of n=-(MaxVarintLen64+1)
which is -11, as per the function signature.
Updated some tests to reflect the new change in expectations of n
when the number of bytes to be read exceeds the limits..
Fixes#41185
Change-Id: Ie346457b1ddb0214b60c72e81128e24d604d083d
Reviewed-on: https://go-review.googlesource.com/c/go/+/299531
Run-TryBot: Emmanuel Odeke <emmanuel@orijtech.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Trust: Emmanuel Odeke <emmanuel@orijtech.com>
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>
The IsExported method is a more intuitive helper for checking whether
the method or field is exported than checking whether PkgPath is empty.
In the same CL, modify the standard library to make use of this helper.
Fixes#41563
Change-Id: Iaacfb3b74449501f98e2707aa32095a32bd3c3c1
Reviewed-on: https://go-review.googlesource.com/c/go/+/266197
Trust: Joe Tsai <joetsai@digital-static.net>
Run-TryBot: Joe Tsai <thebrokentoaster@gmail.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
The decodeState type is a large part of the allocated space during Unmarshal.
The errorContext field is infrequently used, and only on error.
Extract it into a pointer and allocate it separate when necessary.
name old time/op new time/op delta
UnmarshalString-8 115ns ± 5% 114ns ± 3% ~ (p=0.170 n=15+15)
UnmarshalFloat64-8 113ns ± 2% 106ns ± 1% -6.42% (p=0.000 n=15+14)
UnmarshalInt64-8 93.3ns ± 1% 86.9ns ± 4% -6.89% (p=0.000 n=14+15)
name old alloc/op new alloc/op delta
UnmarshalString-8 192B ± 0% 160B ± 0% -16.67% (p=0.000 n=15+15)
UnmarshalFloat64-8 180B ± 0% 148B ± 0% -17.78% (p=0.000 n=15+15)
UnmarshalInt64-8 176B ± 0% 144B ± 0% -18.18% (p=0.000 n=15+15)
name old allocs/op new allocs/op delta
UnmarshalString-8 2.00 ± 0% 2.00 ± 0% ~ (all equal)
UnmarshalFloat64-8 2.00 ± 0% 2.00 ± 0% ~ (all equal)
UnmarshalInt64-8 1.00 ± 0% 1.00 ± 0% ~ (all equal)
Change-Id: I53f3f468e6c65f77a12e5138a2626455b197012d
Reviewed-on: https://go-review.googlesource.com/c/go/+/271338
Trust: Josh Bleecher Snyder <josharian@gmail.com>
Trust: Daniel Martí <mvdan@mvdan.cc>
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
Make all our package sources use Go 1.17 gofmt format
(adding //go:build lines).
Part of //go:build change (#41184).
See https://golang.org/design/draft-gobuild
Change-Id: Ia0534360e4957e58cd9a18429c39d0e32a6addb4
Reviewed-on: https://go-review.googlesource.com/c/go/+/294430
Trust: Russ Cox <rsc@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Jason A. Donenfeld <Jason@zx2c4.com>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
This reverts commit 6af088bfc6.
Reason for revert: Broke many tests inside Google which implies many
tests were broken outside of Google as well. The tests may be brittle
but still would require work to change and it's not clear it's worth
the benefit.
Updates #36221Fixes#42675
Change-Id: Id3a14eb37e7119f5abe50e80dfbf120fdc44db72
Reviewed-on: https://go-review.googlesource.com/c/go/+/273747
Run-TryBot: Joe Tsai <thebrokentoaster@gmail.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
Trust: Joe Tsai <thebrokentoaster@gmail.com>
Fixes the check for the reserved namespace prefix
"xml" to be case insensitive, so as to match all variants of:
(('X'|'x')('M'|'m')('L'|'l'))
as mandated by Section 2.3 of https://www.w3.org/TR/REC-xml/
This is a roll forward of CL 203417, which was rolled back by CL 240179.
We've decided that the roll back was incorrect, and any broken tests
should be fixed.
The original CL 203417 was by Tamás Gulácsi.
Fixes#35151
For #39876
Change-Id: I2e6daa7aeb252531fba0b8a56086613e13059528
Reviewed-on: https://go-review.googlesource.com/c/go/+/264024
Trust: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Damien Neil <dneil@google.com>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
The other named errors - UnmarshalTypeError, etc - in this package do
the same, so we should prepend the package prefix to error messages
for consistency.
Add a note to the release docs in case this is interpreted as
a breaking change.
Fixes#36221.
Change-Id: Ie24b532bbf9812e108c259fa377e2a6b64319ed4
Reviewed-on: https://go-review.googlesource.com/c/go/+/263619
Run-TryBot: Kevin Burke <kev@inburke.com>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Kevin Burke <kev@inburke.com>
Trust: Daniel Martí <mvdan@mvdan.cc>
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
The old ioutil references are still valid, but update our code
to reflect best practices and get used to the new locations.
Code compiled with the bootstrap toolchain
(cmd/asm, cmd/dist, cmd/compile, debug/elf)
must remain Go 1.4-compatible and is excluded.
Also excluded vendored code.
For #41190.
Change-Id: I6d86f2bf7bc37a9d904b6cee3fe0c7af6d94d5b1
Reviewed-on: https://go-review.googlesource.com/c/go/+/263142
Trust: Russ Cox <rsc@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
This change clarifies the usage of the SET type name suffix. Previously
the documentation was somewhat confusing about where the suffix should
be used, and when used what it applied to. For instance the previous
language could be interpreted such that []exampleSET would be parsed as
a SEQUENCE OF SET, which is incorrect as the SET suffix only applies to
slice types, such as type exampleSET []struct{} which is parsed as a
SET OF SEQUENCE.
Change-Id: I74201d9969f931f69391c236559f66cb460569ec
GitHub-Last-Rev: d0d2ddc587
GitHub-Pull-Request: golang/go#38543
Reviewed-on: https://go-review.googlesource.com/c/go/+/229078
Trust: Roland Shoemaker <roland@golang.org>
Reviewed-by: Filippo Valsorda <filippo@golang.org>
Changes Unmarshal to return an error, instead of
panicking when its value is nil or not a pointer.
This change matches the behavior of other encoding
packages like json.
Fixes#41509.
Change-Id: I92c3af3a960144566e4c2b55d00c3a6fe477c8d5
GitHub-Last-Rev: c668b6e4ad
GitHub-Pull-Request: golang/go#41485
Reviewed-on: https://go-review.googlesource.com/c/go/+/255881
Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
Reviewed-by: Filippo Valsorda <filippo@golang.org>
Trust: Emmanuel Odeke <emm.odeke@gmail.com>
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: 6f874944f4
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>
Allow ';' as a valid character for json field keys and struct tags.
Fixes#39189
Change-Id: I4b602a1b0674ff028db07623682f0d1e8e9fd6c9
Reviewed-on: https://go-review.googlesource.com/c/go/+/234818
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Giovanni Bajo <rasky@develer.com>
Trust: Daniel Martí <mvdan@mvdan.cc>
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
This reverts CL 253037.
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: I62896717aa47ed491c2c4775d2b05d80e5e9cde3
Reviewed-on: https://go-review.googlesource.com/c/go/+/254837
Trust: Damien Neil <dneil@google.com>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
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: 930cda5384
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>
Allows users to check:
errors.Is(err, &json.SyntaxError{})
which is the recommended way of checking for kinds of errors.
Change-Id: I20dc805f20212765e9936a82d9cb7822e73ec4ef
GitHub-Last-Rev: e2627ccf8e
GitHub-Pull-Request: golang/go#41210
Reviewed-on: https://go-review.googlesource.com/c/go/+/253037
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Replace strings.Split by strings.IndexByte and explicit
slicing to avoid the allocation of the return slice
of strings.Split.
name old time/op new time/op delta
Marshal 43.3µs ± 1% 36.7µs ± 1% -15.23% (p=0.000 n=9+9)
name old alloc/op new alloc/op delta
Marshal 10.7kB ± 0% 9.2kB ± 0% -13.96% (p=0.000 n=10+10)
name old allocs/op new allocs/op delta
Marshal 444 ± 0% 366 ± 0% -17.57% (p=0.000 n=10+10)
Change-Id: I9e727defa23f7e5fc684f246de0136fe28cf8d25
Reviewed-on: https://go-review.googlesource.com/c/go/+/231738
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
This CL ensures that ReadUvarint consumes only a limited
amount of input (instead of an unbounded amount).
On some inputs, ReadUvarint could read an arbitrary number
of bytes before deciding to return an overflow error.
After this CL, ReadUvarint returns that same overflow
error sooner, after reading at most MaxVarintLen64 bytes.
Fix authored by Robert Griesemer and Filippo Valsorda.
Thanks to Diederik Loerakker, Jonny Rhea, Raúl Kripalani,
and Preston Van Loon for reporting this.
Fixes#40618
Fixes CVE-2020-16845
Change-Id: Ie0cb15972f14c38b7cf7af84c45c4ce54909bb8f
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/812099
Reviewed-by: Filippo Valsorda <valsorda@google.com>
Reviewed-on: https://go-review.googlesource.com/c/go/+/247120
Run-TryBot: Katie Hockman <katie@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Alexander Rakoczy <alex@golang.org>
This reverts https://golang.org/cl/191783.
Reason for revert: Broke too many programs which depended on the previous
behavior, even when it was the opposite of what the documentation said.
We can attempt to fix the original issue again for 1.16, while keeping
those programs in mind.
Fixes#39427.
Change-Id: I7a7f24b2a594c597ef625aeff04fff29aaa88fc6
Reviewed-on: https://go-review.googlesource.com/c/go/+/240657
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
This reverts CL 203417.
Reason for revert: This change changes uses of tags like "XMLSchema-instance" without any recourse.
For #35151Fixes#39876
Change-Id: I4c85c8267a46b3748664b5078794dafffb42aa26
Reviewed-on: https://go-review.googlesource.com/c/go/+/240179
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
Reviewed-by: Andrew Bonventre <andybons@golang.org>
This reverts golang.org/cl/190659 and golang.org/cl/226218, minus the
regression tests in the latter.
The original work happened in golang.org/cl/151157, which was reverted
in golang.org/cl/190909 due to a crash found by fuzzing.
We tried a second time in golang.org/cl/190659, which shipped with Go
1.14. A bug was found, where strings would be mangled in certain edge
cases. The fix for that was golang.org/cl/226218, which was backported
into Go 1.14.4.
Unfortunately, a second regression was just reported in #39555, which is
a similar case of strings getting mangled when decoding under certain
conditions. It would be possible to come up with another small patch to
fix that edge case, but instead, let's just revert the entire
optimization, as it has proved to do more harm than good. Moreover, it's
hard to argue or prove that there will be no more such regressions.
However, all the work wasn't for nothing. First, we learned that the way
the decoder unquotes tokenized strings isn't simple; initially, we had
wrongly assumed that each string was unquoted exactly once and in order.
Second, we have gained a number of regression tests which will be useful
to prevent the same mistakes in the future, including the test cases we
add in this CL.
Fixes#39555.
Change-Id: I66a6919c2dd6d9789232482ba6cf3814eaa70f61
Reviewed-on: https://go-review.googlesource.com/c/go/+/237838
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Andrew Bonventre <andybons@golang.org>
fieldInfo.value used to initialize nil anonymous struct fields if they
were encountered. This behavior is wanted when decoding, but not when
encoding. When encoding, the value should never be modified, and these
nil fields should be skipped entirely.
To fix the bug, add a bool argument to the function which tells the
code whether we are encoding or decoding.
Finally, add a couple of tests to cover the edge cases pointed out in
the original issue.
Fixes#27240.
Change-Id: Ic97ae4bfe5f2062c8518e03d1dec07c3875e18f6
Reviewed-on: https://go-review.googlesource.com/c/go/+/196809
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
This reverts golang.org/cl/179337.
Reason for revert: broke a few too many reasonably valid Go programs.
The previous behavior was perhaps less consistent, but the docs were
never very clear about when the decoder merges with existing values,
versus replacing existing values altogether.
Fixes#39149.
Change-Id: I1c1d857709b8398969fe421aa962f6b62f91763a
Reviewed-on: https://go-review.googlesource.com/c/go/+/234559
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Andrew Bonventre <andybons@golang.org>
Specifically, this change documents the behavior of Unmarshal when a
SEQUENCE contains trailing elements.
For context Unmarshal treats trailing elements of a SEQUENCE that do not
have matching struct fields as valid, as this is how ASN.1 structures
are typically extended. This can be somewhat confusing as you might
expect those elements to be appended to rest, but rest is really only
for trailing data unrelated to the structure being parsed (i.e. if you
append a second sequence to b, it would be returned in rest).
Fixes#35680
Change-Id: Ia2c68b2f7d8674d09e859b4b7f9aff327da26fa0
Reviewed-on: https://go-review.googlesource.com/c/go/+/233537
Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Katie Hockman <katie@golang.org>
When we decode into a struct, each input key-value may be decoded into
one of the struct's fields. Particularly, existing data isn't dropped,
so that some sub-fields can be decoded into without zeroing all other
data.
However, decoding into a map behaved in the opposite way. Whenever a
key-value was decoded, it completely replaced the previous map element.
If the map contained any non-zero data in that key, it's dropped.
Instead, try to reuse the existing element value if possible. If the map
element type is a pointer, and the value is non-nil, we can decode
directly into it. If it's not a pointer, make a copy and decode into
that copy, as map element values aren't addressable.
This means we have to parse and convert the map element key before the
value, to be able to obtain the existing element value. This is fine,
though. Moreover, reporting errors on the key before the value follows
the input order more closely.
Finally, add a test to explore the four combinations, involving pointer
and non-pointer, and non-zero and zero values. A table-driven test
wasn't used, as each case required different checks, such as checking
that the non-nil pointer case doesn't end up with a different pointer.
Fixes#31924.
Change-Id: I5ca40c9963a98aaf92f26f0b35843c021028dfca
Reviewed-on: https://go-review.googlesource.com/c/go/+/179337
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
The added comment contains some context. The original optimization
assumed that each call to unquoteBytes (or unquote) followed its
corresponding call to rescanLiteral. Otherwise, unquoting a literal
might use d.safeUnquote from another re-scanned literal.
Unfortunately, this assumption is wrong. When decoding {"foo": "bar"}
into a map[T]string where T implements TextUnmarshaler, the sequence of
calls would be as follows:
1) rescanLiteral "foo"
2) unquoteBytes "foo"
3) rescanLiteral "bar"
4) unquoteBytes "foo" (for UnmarshalText)
5) unquoteBytes "bar"
Note that the call to UnmarshalText happens in literalStore, which
repeats the work to unquote the input string literal. But, since that
happens after we've re-scanned "bar", we're using the wrong safeUnquote
field value.
In the added test case, the second string had a non-zero number of safe
bytes, and the first string had none since it was all non-ASCII. Thus,
"safely" unquoting a number of the first string's bytes could cut a rune
in half, and thus mangle the runes.
A rather simple fix, without a full revert, is to only allow one use of
safeUnquote per call to unquoteBytes. Each call to rescanLiteral when
we have a string is soon followed by a call to unquoteBytes, so it's no
longer possible for us to use the wrong index.
Also add a test case from #38126, which is the same underlying bug, but
affecting the ",string" option.
Before the fix, the test would fail, just like in the original two issues:
--- FAIL: TestUnmarshalRescanLiteralMangledUnquote (0.00s)
decode_test.go:2443: Key "开源" does not exist in map: map[开���:12345开源]
decode_test.go:2458: Unmarshal unexpected error: json: invalid use of ,string struct tag, trying to unmarshal "\"aaa\tbbb\"" into string
Fixes#38105.
For #38126.
Change-Id: I761e54924e9a971a4f9eaa70bbf72014bb1476e6
Reviewed-on: https://go-review.googlesource.com/c/go/+/226218
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Joe Tsai <thebrokentoaster@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>
Per X690 Section 11.6 sort the order of SET of components when generating
DER. This CL makes no changes to Unmarshal, meaning unordered components
will still be accepted, and won't be re-ordered during parsing.
In order to sort the components a new encoder, setEncoder, which is similar
to multiEncoder is added. The functional difference is that setEncoder
encodes each component to a [][]byte, sorts the slice using a sort.Sort
interface, and then writes it out to the destination slice. The ordering
matches the output of OpenSSL.
Fixes#24254
Change-Id: Iff4560f0b8c2dce5aae616ba30226f39c10b972e
GitHub-Last-Rev: e52fc43658
GitHub-Pull-Request: golang/go#38228
Reviewed-on: https://go-review.googlesource.com/c/go/+/226984
Reviewed-by: Filippo Valsorda <filippo@golang.org>
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reject base 128 encoded integers that aren't using minimal encoding,
specifically if the leading octet of an encoded integer is 0x80. This
only affects parsing of tags and OIDs, both of which expect this
encoding (see X.690 8.1.2.4.2 and 8.19.2).
Fixes#36881
Change-Id: I969cf48ac1fba7e56bac334672806a0784d3e123
GitHub-Last-Rev: fefc03d202
GitHub-Pull-Request: golang/go#38281
Reviewed-on: https://go-review.googlesource.com/c/go/+/227320
Reviewed-by: Filippo Valsorda <filippo@golang.org>
Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The previous behavior directly contradicted the docs that have been in
place for years:
To unmarshal a JSON array into a slice, Unmarshal resets the
slice length to zero and then appends each element to the slice.
We could use reflect.New to create a new element and reflect.Append to
then append it to the destination slice, but benchmarks have shown that
reflect.Append is very slow compared to the code that manually grows a
slice in this file.
Instead, if we're decoding into an element that came from the original
backing array, zero it before decoding into it. We're going to be using
the CodeDecoder benchmark, as it has a slice of struct pointers that's
decoded very often.
Note that we still reuse existing values from arrays being decoded into,
as the documentation agrees with the existing implementation in that
case:
To unmarshal a JSON array into a Go array, Unmarshal decodes
JSON array elements into corresponding Go array elements.
The numbers with the benchmark as-is might seem catastrophic, but that's
only because the benchmark is decoding into the same variable over and
over again. Since the old decoder was happy to reuse slice elements, it
would save a lot of allocations by not having to zero and re-allocate
said elements:
name old time/op new time/op delta
CodeDecoder-8 10.4ms ± 1% 10.9ms ± 1% +4.41% (p=0.000 n=10+10)
name old speed new speed delta
CodeDecoder-8 186MB/s ± 1% 178MB/s ± 1% -4.23% (p=0.000 n=10+10)
name old alloc/op new alloc/op delta
CodeDecoder-8 2.19MB ± 0% 3.59MB ± 0% +64.09% (p=0.000 n=10+10)
name old allocs/op new allocs/op delta
CodeDecoder-8 76.8k ± 0% 92.7k ± 0% +20.71% (p=0.000 n=10+10)
We can prove this by moving 'var r codeResponse' into the loop, so that
the benchmark no longer reuses the destination pointer. And sure enough,
we no longer see the slow-down caused by the extra allocations:
name old time/op new time/op delta
CodeDecoder-8 10.9ms ± 0% 10.9ms ± 1% -0.37% (p=0.043 n=10+10)
name old speed new speed delta
CodeDecoder-8 177MB/s ± 0% 178MB/s ± 1% +0.37% (p=0.041 n=10+10)
name old alloc/op new alloc/op delta
CodeDecoder-8 3.59MB ± 0% 3.59MB ± 0% ~ (p=0.780 n=10+10)
name old allocs/op new allocs/op delta
CodeDecoder-8 92.7k ± 0% 92.7k ± 0% ~ (all equal)
I believe that it's useful to leave the benchmarks as they are now,
because the decoder does reuse memory in some cases. For example,
existing map elements are reused. However, subtle changes like this one
need to be benchmarked carefully.
Finally, add a couple of tests involving both a slice and an array of
structs.
Fixes#21092.
Change-Id: I8b1194f25e723a31abd146fbfe9428ac10c1389d
Reviewed-on: https://go-review.googlesource.com/c/go/+/191783
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Fixes the check for the reserved namespace prefix
"xml" to be case insensitive, so as to match all variants of:
(('X'|'x')('M'|'m')('L'|'l'))
as mandated by Section 2.3 of https://www.w3.org/TR/REC-xml/Fixes#35151.
Change-Id: Id5a98e5f9d69d3741dc16f567c4320f1ad0b3c70
Reviewed-on: https://go-review.googlesource.com/c/go/+/203417
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Change-Id: I1fd47e5eab27346cec488098d4f6102a0749bd28
Reviewed-on: https://go-review.googlesource.com/c/go/+/221788
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
This CL changes some unit test functions, making sure that these tests (and goroutines spawned during test) won't block.
Since they are just test functions, I use one CL to fix them all. I hope this won't cause trouble to reviewers and can save time for us.
There are three main categories of incorrect logic fixed by this CL:
1. Use testing.Fatal()/Fatalf() in spawned goroutines, which is forbidden by Go's document.
2. Channels are used in such a way that, when errors or timeout happen, the test will be blocked and never return.
3. Channels are used in such a way that, when errors or timeout happen, the test can return but some spawned goroutines will be leaked, occupying resource until all other tests return and the process is killed.
Change-Id: I3df931ec380794a0cf1404e632c1dd57c65d63e8
Reviewed-on: https://go-review.googlesource.com/c/go/+/219380
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Instead use string(r) where r has type rune.
This is in preparation for a vet warning for string(i).
Updates #32479
Change-Id: Ic205269bba1bd41723950219ecfb67ce17a7aa79
Reviewed-on: https://go-review.googlesource.com/c/go/+/220844
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Akhil Indurti <aindurti@gmail.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Toshihiro Shiino <shiino.toshihiro@gmail.com>
Limit the maximum nesting depth when parsing to protect against stack
overflow, permitted by https://tools.ietf.org/html/rfc7159#section-9
A nesting depth limit of 10,000 was chosen to be a conservative
balance between avoiding stack overflow and avoiding impacting
legitimate JSON documents.
10,000 is less than 1% of the experimental stack depth limit
with the default stack size:
* On 64-bit systems, the default stack limit is 1GB,
which allows ~2,800,000 frames of recursive parsing
* On 32-bit systems, the default stack limit is 250MB,
which allows ~1,100,000 frames of recursive parsing
Fixes#31789
Change-Id: I4f5a90e89dcb4ab1a957ad9d02e1fa0efafaccf6
Reviewed-on: https://go-review.googlesource.com/c/go/+/199837
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
This updates the link to a newer image.
Change-Id: Ibdfe8c57d9217a325bcfde98cb6f952ca63d588a
GitHub-Last-Rev: f5970ba395
GitHub-Pull-Request: golang/go#36938
Reviewed-on: https://go-review.googlesource.com/c/go/+/217297
Reviewed-by: Toshihiro Shiino <shiino.toshihiro@gmail.com>
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
After golang.org/cl/210124, I wondered if the same error had gone
unnoticed elsewhere. I quickly spotted another dozen mistakes after
reading through the output of:
git grep '\<[Aa]n [bcdfgjklmnpqrtvwyz][a-z]'
Many results are false positives for acronyms like "an mtime", since
it's pronounced "an em-time". However, the total amount of output isn't
that large given how simple the grep pattern is.
Change-Id: Iaa2ca69e42f4587a9e3137d6c5ed758887906ca6
Reviewed-on: https://go-review.googlesource.com/c/go/+/210678
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Zach Jones <zachj1@gmail.com>
Reviewed-by: Robert Griesemer <gri@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
An application that wants to reject non-canonical encodings is likely to
care about other sources of malleability.
Change-Id: I1d3a5b281d2631ca78df3f89b957a02687a534d8
Reviewed-on: https://go-review.googlesource.com/c/go/+/188858
Reviewed-by: Katie Hockman <katie@golang.org>
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>
Make binary.Read return an error when passed `data` argument is not
a pointer to a fixed-size value or a slice of fixed-size values.
Fixes#32927
Change-Id: I04f48be55fe9b0cc66c983d152407d0e42cbcd95
Reviewed-on: https://go-review.googlesource.com/c/go/+/184957
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
A majority of work is spent in dataSize when en/decoding the same
struct over and over again. This wastes a lot of work, since
the result doesn't change for a given reflect.Value.
Cache the result of the function for structs, so that subsequent
calls to dataSize can avoid doing work.
name old time/op new time/op delta
ReadStruct 1.00µs ± 1% 0.37µs ± 1% -62.99% (p=0.029 n=4+4)
WriteStruct 1.00µs ± 3% 0.37µs ± 1% -62.69% (p=0.008 n=5+5)
name old speed new speed delta
ReadStruct 75.1MB/s ± 1% 202.9MB/s ± 1% +170.16% (p=0.029 n=4+4)
WriteStruct 74.8MB/s ± 3% 200.4MB/s ± 1% +167.96% (p=0.008 n=5+5)
Fixes#34471
Change-Id: Ic5d987ca95f1197415ef93643a0af6fc1224fdf0
Reviewed-on: https://go-review.googlesource.com/c/go/+/199539
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This is a re-submission of CL 151157, since it was reverted in CL 190909
due to an introduced crash found by a fuzzer. The revert CL included
regression tests, while this CL includes a fixed version of the original
change.
In particular, what we forgot in the original optimization was that we
still need the length and trailing quote checks at the beginning of
unquoteBytes. Without those, we could end up in a crash later on.
We can work out how many bytes can be unquoted trivially in
rescanLiteral, which already iterates over a string's bytes.
Removing the extra loop in unquoteBytes simplifies the function and
speeds it up, especially when decoding simple strings, which are common.
While at it, we can remove the check that s[0]=='"', since all call
sites already meet that condition.
name old time/op new time/op delta
CodeDecoder-8 10.6ms ± 2% 10.5ms ± 1% -1.01% (p=0.004 n=20+10)
name old speed new speed delta
CodeDecoder-8 183MB/s ± 2% 185MB/s ± 1% +1.02% (p=0.003 n=20+10)
Updates #28923.
Change-Id: I8c6b13302bcd86a364bc998d72451332c0809cde
Reviewed-on: https://go-review.googlesource.com/c/go/+/190659
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Peter Weinberger <pjw@google.com>
The documentation for TokenReader suggests that implementations of the
interface may return a token and io.EOF together, indicating that it is
the last token in the stream. This is similar to io.Reader. However, if
you wrap such a TokenReader in a Decoder it complained about the EOF.
A test was added to ensure this behavior on Decoder's.
Change-Id: I9083c91d9626180d3bcf5c069a017050f3c7c4a8
Reviewed-on: https://go-review.googlesource.com/c/go/+/130556
Run-TryBot: Sam Whited <sam@samwhited.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
If we marshal a non-pointer struct field whose type implements Marshaler with
a non-pointer receiver, then we avoid an allocation if we take the address of
the field before casting it to an interface.
name old time/op new time/op delta
EncodeMarshaler-8 104ns ± 1% 92ns ± 2% -11.72% (p=0.001 n=7+7)
name old alloc/op new alloc/op delta
EncodeMarshaler-8 36.0B ± 0% 4.0B ± 0% -88.89% (p=0.000 n=8+8)
name old allocs/op new allocs/op delta
EncodeMarshaler-8 2.00 ± 0% 1.00 ± 0% -50.00% (p=0.000 n=8+8)
Test coverage already looks good enough for this change. TestRefValMarshal
already covers all possible combinations of value & pointer receivers on
value and pointer struct fields.
Change-Id: I6fc7f72396396d98f9a90c3c86e813690f41c099
Reviewed-on: https://go-review.googlesource.com/c/go/+/203608
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This change improves performance of Compact by using a sync.Pool to allow re-use
of a scanner. This also has the side-effect of removing an allocation for each
field that implements Marshaler when marshalling JSON.
name old time/op new time/op delta
EncodeMarshaler-8 118ns ± 2% 104ns ± 1% -12.21% (p=0.001 n=7+7)
name old alloc/op new alloc/op delta
EncodeMarshaler-8 100B ± 0% 36B ± 0% -64.00% (p=0.000 n=8+8)
name old allocs/op new allocs/op delta
EncodeMarshaler-8 3.00 ± 0% 2.00 ± 0% -33.33% (p=0.000 n=8+8)
Change-Id: Ic70c61a0a6354823da5220f5aad04b94c054f233
Reviewed-on: https://go-review.googlesource.com/c/go/+/200864
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@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>
This makes Decoder.offset public while renaming it to
Decoder.InputOffset to match encoding/xml Decoder API
Code changes made by Adam Stankiewicz [sheerun@sher.pl]
Fixes#29688
Change-Id: I86dbfd2b2da80160846e92bfa580c53d8d45e2db
Reviewed-on: https://go-review.googlesource.com/c/go/+/200677
Run-TryBot: Johan Brandhorst <johan.brandhorst@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
When unmarshaling to a map, the map's key type must either be a string,
an integer, or implement encoding.TextUnmarshaler. But for a user
defined type, reflect.Kind will not distinguish between the static type
and the underlying type. In:
var x MyString = "x"
t := reflect.TypeOf(x)
println(t.Kind() == reflect.String)
the Kind of x is still reflect.String, even though the static type of x
is MyString.
Moreover, checking for the map's key type is a string occurs first, so
even if the map key type MyString implements encoding.TextUnmarshaler,
it will be ignored.
To fix the bug, check for encoding.TextUnmarshaler first.
Fixes#34437
Change-Id: I780e0b084575e1dddfbb433fe03857adf71d05fb
Reviewed-on: https://go-review.googlesource.com/c/go/+/200237
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
Compact has been inconsistently escaping only some problematic characters
(U+2028 and U+2029), but not others (<, > and &). This change addresses
this inconsistency by removing the escaping of U+2028 and U+2029.
Callers who need to escape the output of Compact should use HTMLEscape
which escapes <, >, &, U+2028 and U+2029.
Fixes#34070Fixes#30357
Updates #5836
Change-Id: Icfce7691d2b8b1d9b05ba7b64d2d1e4f3b67871b
GitHub-Last-Rev: 38859fe3e2
GitHub-Pull-Request: golang/go#34804
Reviewed-on: https://go-review.googlesource.com/c/go/+/200217
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Unmarshaling a string into a json.Number should first check that the string is a valid Number.
If not, we should fail without decoding it.
Fixes#14702
Change-Id: I286178e93df74ad63c0a852c3f3489577072cf47
GitHub-Last-Rev: fe69bb68ee
GitHub-Pull-Request: golang/go#34272
Reviewed-on: https://go-review.googlesource.com/c/go/+/195045
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@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: dde81b7120
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>
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.
Not using Marshal here also removes the error check as there has not been a
way for Marshal to fail anyway.
name old time/op new time/op delta
Issue34127-4 360ns ± 3% 200ns ± 3% -44.56% (p=0.008 n=5+5)
name old alloc/op new alloc/op delta
Issue34127-4 56.0B ± 0% 40.0B ± 0% -28.57% (p=0.008 n=5+5)
name old allocs/op new allocs/op delta
Issue34127-4 3.00 ± 0% 2.00 ± 0% -33.33% (p=0.008 n=5+5)
Fixes#34154
Change-Id: Ib60dc11980f9b20d8bef2982de7168943d632263
GitHub-Last-Rev: 9b0ac1d4c5
GitHub-Pull-Request: golang/go#34127
Reviewed-on: https://go-review.googlesource.com/c/go/+/193604
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reset is already performed when retrieving from pool
Change-Id: Ia810dd18d3e55a1565a5ad435a00d1e46724576c
GitHub-Last-Rev: d9df74a4ae
GitHub-Pull-Request: golang/go#34195
Reviewed-on: https://go-review.googlesource.com/c/go/+/194338
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
The indirect method checked the type of the child when indirecting a
pointer. If the current value is a pointer and we are decoding null, we
can skip this entirely and return early, avoiding the whole descent.
Fixes#31776
Change-Id: Ib8b2a2357572c41f56fceac59b5a858980f3f65e
Reviewed-on: https://go-review.googlesource.com/c/go/+/174699
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
This code enables handling of ASN1's string type BMPString, used in some digital signatures.
Parsing code taken from golang.org/x/crypto/pkcs12.
Change-Id: Ibeae9cf4d8ae7c18f8b5420ad9244a16e117ff6b
GitHub-Last-Rev: 6945253514
GitHub-Pull-Request: golang/go#26690
Reviewed-on: https://go-review.googlesource.com/c/go/+/126624
Run-TryBot: Andrew Bonventre <andybons@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Andrew Bonventre <andybons@golang.org>
This is a documentation-only change
Fixes#33298
Change-Id: I816058a872b57dc868dff11887214d9de92d9342
Reviewed-on: https://go-review.googlesource.com/c/go/+/188821
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Use the following (suboptimal) script to obtain a list of possible
typos:
#!/usr/bin/env sh
set -x
git ls-files |\
grep -e '\.\(c\|cc\|go\)$' |\
xargs -n 1\
awk\
'/\/\// { gsub(/.*\/\//, ""); print; } /\/\*/, /\*\// { gsub(/.*\/\*/, ""); gsub(/\*\/.*/, ""); }' |\
hunspell -d en_US -l |\
grep '^[[:upper:]]\{0,1\}[[:lower:]]\{1,\}$' |\
grep -v -e '^.\{1,4\}$' -e '^.\{16,\}$' |\
sort -f |\
uniq -c |\
awk '$1 == 1 { print $2; }'
Then, go through the results manually and fix the most obvious typos in
the non-vendored code.
Change-Id: I3cb5830a176850e1a0584b8a40b47bde7b260eae
Reviewed-on: https://go-review.googlesource.com/c/go/+/193848
Reviewed-by: Robert Griesemer <gri@golang.org>
Some were never used, and some haven't been used for years.
One exception is net/http's readerAndCloser, which was only used in a
test. Move it to a test file.
While at it, remove a check in regexp that could never fire; the field
is an uint32, so it can never be negative.
Change-Id: Ia2200f6afa106bae4034045ea8233b452f38747b
Reviewed-on: https://go-review.googlesource.com/c/go/+/192621
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
scanEnd is delayed one byte so we decrement
the scanner bytes count by 1 to ensure that
this value is correct in the next call of Decode.
Fixes#32399
Change-Id: I8c8698e7f95bbcf0373aceaa05319819eae9d86f
GitHub-Last-Rev: 0ac25d8de2
GitHub-Pull-Request: golang/go#32598
Reviewed-on: https://go-review.googlesource.com/c/go/+/182117
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This partly reverts CL 173417 as it incorrectly documented that Compact
performed HTML escaping and the output was safe to embed inside HTML
<script> tags. This has never been true.
Although Compact does escape U+2028 and U+2029, it doesn't escape <, >
or &. Compact is thus only performing a subset of HTML escaping and it's
output is not safe to embed inside HTML <script> tags.
A more complete fix would be for Compact to either never perform any
HTML escaping, as it was prior to CL 10883045, or to actually perform
the same HTML escaping as HTMLEscape. Neither change is likely safe
enough for go1.13.
Updates #30357
Change-Id: I912f0fe9611097d988048b28228c4a5b985080ba
GitHub-Last-Rev: aebababc92
GitHub-Pull-Request: golang/go#33427
Reviewed-on: https://go-review.googlesource.com/c/go/+/188717
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: 6c987c9084
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>
Per the code review guidelines: "Words in names that are
initialisms or acronyms have a consistent case."
Change-Id: I347b02d2f48455f2cbbc040191ba197e3e8f23fc
Reviewed-on: https://go-review.googlesource.com/c/go/+/191970
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The decoder called this function to check numbers being decoded into a
json.Number. However, these can't be quoted as strings, so the tokenizer
has already verified they are valid JSON numbers.
Verified this by adding a test with such an input. As expected, it
produces a syntax error, not the fmt.Errorf - that line could never
execute.
Since the only remaining non-test caller of isvalidnumber is in
encode.go, move the function there.
This change should slightly reduce the amount of work when decoding into
json.Number, though that isn't very common nor part of any current
benchmarks.
Change-Id: I67a1723deb3d18d5b542d6dd35f3ae56a43f23eb
Reviewed-on: https://go-review.googlesource.com/c/go/+/184817
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Because TestUnmarshal actually allocates a new value to decode into
using ptr's pointer type, any existing data is thrown away. This was
harmless in alomst all of the test cases, minus the "overwriting of
data" ones added in 2015 in CL 12209.
I spotted that nothing covered decoding a JSON array with few elements
into a slice which already had many elements. I initially assumed that
the code was buggy or that some code could be removed, when in fact
there simply wasn't any code covering the edge case.
Move those two tests to TestPrefilled, which already served a very
similar purpose. Remove the map case, as TestPrefilled already has
plenty of prefilled map cases. Moreover, we no longer reset an entire
map when decoding, as per the godoc:
To unmarshal a JSON object into a map, Unmarshal first
establishes a map to use. If the map is nil, Unmarshal allocates
a new map. Otherwise Unmarshal reuses the existing map, keeping
existing entries.
Finally, to ensure that ptr is used correctly in the future, make
TestUnmarshal error if it's anything other than a pointer to a zero
value. That is, the only correct use should be new(type). Don't rename
the ptr field, as that would be extremely noisy and cause unwanted merge
conflicts.
Change-Id: I41e3ecfeae42d877ac5443a6bd622ac3d6c8120c
Reviewed-on: https://go-review.googlesource.com/c/go/+/185738
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
This reverts CL 151157.
CL 151157 introduced a crash when decoding into ",string" fields. It
came with a moderate speedup, so at this stage of the release cycle
let's just revert it, and reapply it in Go 1.14 with the fix in CL 190659.
Also applied the test cases from CL 190659.
Updates #33728
Change-Id: Ie46e2bc15224b251888580daf6b79d5865f3878e
Reviewed-on: https://go-review.googlesource.com/c/go/+/190909
Run-TryBot: Andrew Bonventre <andybons@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Andrew Bonventre <andybons@golang.org>
Currently test build fails with:
$ go test -tags=gofuzz encoding/json
encoding/json/fuzz.go:36:4: Println call has possible formatting directive %s
FAIL encoding/json [build failed]
Change-Id: I23aef44a421ed0e7bcf48b74ac5a8c6768a4841b
Reviewed-on: https://go-review.googlesource.com/c/go/+/190698
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
It wasn't obeyed in the case where the MarshalJSON method uses a pointer
receiver, and the encoder grabs the address of a value to find that
method. addrMarshalerEncoder is the function that does this work, but it
ignored opts.escapeHTML.
Here's the before and after of the added test case, which was failing
before the fix. Now the two cases are correct and consistent.
{"NonPtr":"<str>","Ptr":"\u003cstr\u003e"}
{"NonPtr":"<str>","Ptr":"<str>"}
Fixes#32896.
Change-Id: Idc53077ece074973558bd3bb5ad036380db0d02c
Reviewed-on: https://go-review.googlesource.com/c/go/+/184757
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
Reviewed-by: Caleb Spare <cespare@gmail.com>
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>
Gerrit is complaining about pushes that affect these files
and forcing people to use -o nokeycheck, which defeats
the point of the check. Hide the keys from this kind of scan
by marking them explicitly as testing keys.
This is a little annoying but better than training everyone
who ever edits one of these test files to reflexively override
the Gerrit check.
The only remaining keys explicitly marked as private instead
of testing are in examples, and there's not much to do
about those. Hopefully they are not edited as much.
Change-Id: I4431592b5266cb39fe6a80b40e742d97da803a0b
Reviewed-on: https://go-review.googlesource.com/c/go/+/178178
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Renaming the method makes clear, both to readers and to vet,
that this method is not the implementation of io.ByteWriter.
Working toward making the tree vet-safe instead of having
so many exceptions in cmd/vet/all/whitelist.
For #31916.
Change-Id: I5b509eb7f0118d5f2d3c6e352ff2849cd5a3071e
Reviewed-on: https://go-review.googlesource.com/c/go/+/176110
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Most changes are removing redundant declaration of type when direct
instantiating value of map or slice, e.g. []T{T{}} become []T{{}}.
Small changes are removing the high order of subslice if its value
is the length of slice itself, e.g. T[:len(T)] become T[:].
The following file is excluded due to incompatibility with go1.4,
- src/cmd/compile/internal/gc/ssa.go
Change-Id: Id3abb09401795ce1e6da591a89749cba8502fb26
Reviewed-on: https://go-review.googlesource.com/c/go/+/166437
Run-TryBot: Dave Cheney <dave@cheney.net>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Add Unwrap methods to types which wrap an underlying error:
"encodinc/csv".ParseError
"encoding/json".MarshalerError
"net/http".transportReadFromServerError
"net".OpError
"net".DNSConfigError
"net/url".Error
"os/exec".Error
"signal/internal/pty".PtyError
"text/template".ExecError
Add os.ErrTemporary. A case could be made for putting this error
value in package net, since no exported error types in package os
include a Temporary method. However, syscall errors returned from
the os package do include this method.
Add Is methods to error types with a Timeout or Temporary method,
making errors.Is(err, os.Err{Timeout,Temporary}) equivalent to
testing the corresponding method:
"context".DeadlineExceeded
"internal/poll".TimeoutError
"net".adrinfoErrno
"net".OpError
"net".DNSError
"net/http".httpError
"net/http".tlsHandshakeTimeoutError
"net/pipe".timeoutError
"net/url".Error
Updates #30322
Updates #29934
Change-Id: I409fb20c072ea39116ebfb8c7534d493483870dc
Reviewed-on: https://go-review.googlesource.com/c/go/+/170037
Run-TryBot: Damien Neil <dneil@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Marcel van Lohuizen <mpvl@golang.org>
It's slow & often times out randomly on longtest builders. Not useful.
Fixes#31517
Change-Id: Icedbb0c94fbe43d04e8b47d5785ac61c5e2d8750
Reviewed-on: https://go-review.googlesource.com/c/go/+/174522
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
indirect walks down v until it gets to a non-pointer. But it does not
handle the case when v is a pointer to itself, like in:
var v interface{}
v = &v
Unmarshal(b, v)
So just stop immediately if we see v is a pointer to itself.
Fixes#31740
Change-Id: Ie396264119e24d70284cd9bf76dcb2050babb069
Reviewed-on: https://go-review.googlesource.com/c/go/+/174337
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Make explicit that Compact does HTML escaping.
Fixes#30357.
Change-Id: I4648f8f3e907d659db977d07253f716df6e07d7b
Reviewed-on: https://go-review.googlesource.com/c/go/+/173417
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
In the common case, structs have a handful of fields and most inputs
match struct field names exactly.
The previous code would do a linear search over the fields, stopping at
the first exact match, and otherwise using the first case insensitive
match.
This is unfortunate, because it means that for the common case, we'd do
a linear search with bytes.Equal. Even for structs with only two or
three fields, that is pretty wasteful.
Worse even, up until the exact match was found via the linear search,
all previous fields would run their equalFold functions, which aren't
cheap even in the simple case.
Instead, cache a map along with the field list that indexes the fields
by their name. This way, a case sensitive field search doesn't involve a
linear search, nor does it involve any equalFold func calls.
This patch should also slightly speed up cases where there's a case
insensitive match but not a case sensitive one, as then we'd avoid
calling bytes.Equal on all the fields. Though that's not a common case,
and there are no benchmarks for it.
name old time/op new time/op delta
CodeDecoder-8 11.0ms ± 0% 10.6ms ± 1% -4.42% (p=0.000 n=9+10)
name old speed new speed delta
CodeDecoder-8 176MB/s ± 0% 184MB/s ± 1% +4.62% (p=0.000 n=9+10)
name old alloc/op new alloc/op delta
CodeDecoder-8 2.28MB ± 0% 2.28MB ± 0% ~ (p=0.725 n=10+10)
name old allocs/op new allocs/op delta
CodeDecoder-8 76.9k ± 0% 76.9k ± 0% ~ (all equal)
Updates #28923.
Change-Id: I9929c1f06c76505e5b96914199315dbdaae5dc76
Reviewed-on: https://go-review.googlesource.com/c/go/+/172918
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
We can work out how many bytes can be unquoted trivially in
rescanLiteral, which already iterates over a string's bytes.
Removing the extra loop in unquoteBytes simplifies the function and
speeds it up, especially when decoding simple strings, which are common.
While at it, we can remove unnecessary checks like len(s)<2 and
s[0]=='"'. Add a comment explaining why.
name old time/op new time/op delta
CodeDecoder-8 11.2ms ± 0% 11.1ms ± 1% -1.63% (p=0.000 n=9+10)
name old speed new speed delta
CodeDecoder-8 173MB/s ± 0% 175MB/s ± 1% +1.66% (p=0.000 n=9+10)
Updates #28923.
Change-Id: I2436a3a7f8148a2f7a6a4cdbd7dec6b32ef5e20c
Reviewed-on: https://go-review.googlesource.com/c/go/+/151157
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
readValue is a hot function, clocking in at ~13% flat CPU use in
CodeDecoder. In particular, looping over the bytes is slow. That's
partially because the code contains a bounds check at the start of the
loop.
The source of the problem is that scanp is a signed integer, and comes
from a field, so the compiler doesn't know that it's non-negative. Help
it with a simple and comparatively cheap hint.
While at it, use scanp as the index variable directly, removing the need
for a duplicate index variable which is later added back into scanp.
name old time/op new time/op delta
CodeDecoder-8 11.3ms ± 1% 11.2ms ± 1% -0.98% (p=0.000 n=9+9)
name old speed new speed delta
CodeDecoder-8 172MB/s ± 1% 174MB/s ± 1% +0.99% (p=0.000 n=9+9)
Updates #28923.
Change-Id: I138f83babdf316fc97697cc18f595c3403c1ddb7
Reviewed-on: https://go-review.googlesource.com/c/go/+/170939
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
This was the only benchmark missing the SetBytes call, as spotted
earlier by Bryan.
It's not required to make the benchmark useful, but it can still be a
good way to see how its speed is affected by the reduced allocations:
name time/op
CodeUnmarshal-8 12.1ms ± 1%
CodeUnmarshalReuse-8 11.4ms ± 1%
name speed
CodeUnmarshal-8 161MB/s ± 1%
CodeUnmarshalReuse-8 171MB/s ± 1%
name alloc/op
CodeUnmarshal-8 3.28MB ± 0%
CodeUnmarshalReuse-8 1.94MB ± 0%
name allocs/op
CodeUnmarshal-8 92.7k ± 0%
CodeUnmarshalReuse-8 77.6k ± 0%
While at it, remove some unnecessary empty lines.
Change-Id: Ib2bd92d5b3237b8f3092e8c6f863dab548fee2f5
Reviewed-on: https://go-review.googlesource.com/c/go/+/170938
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Decoder.Decode and Unmarshal actually scan the input bytes twice - the
first time to check for syntax errors and the length of the value, and
the second to perform the decoding.
It's in the second scan that we actually tokenize the bytes. Since
syntax errors aren't a possibility, we can take shortcuts.
In particular, literals such as quoted strings are very common in JSON,
so we can avoid a lot of work by special casing them.
name old time/op new time/op delta
CodeDecoder-8 10.3ms ± 1% 9.1ms ± 0% -11.89% (p=0.002 n=6+6)
UnicodeDecoder-8 342ns ± 0% 283ns ± 0% -17.25% (p=0.000 n=6+5)
DecoderStream-8 239ns ± 0% 230ns ± 0% -3.90% (p=0.000 n=6+5)
CodeUnmarshal-8 11.0ms ± 0% 9.8ms ± 0% -11.45% (p=0.002 n=6+6)
CodeUnmarshalReuse-8 10.3ms ± 0% 9.0ms ± 0% -12.72% (p=0.004 n=5+6)
UnmarshalString-8 104ns ± 0% 92ns ± 0% -11.35% (p=0.002 n=6+6)
UnmarshalFloat64-8 93.2ns ± 0% 87.6ns ± 0% -6.01% (p=0.010 n=6+4)
UnmarshalInt64-8 74.5ns ± 0% 71.5ns ± 0% -3.91% (p=0.000 n=5+6)
name old speed new speed delta
CodeDecoder-8 189MB/s ± 1% 214MB/s ± 0% +13.50% (p=0.002 n=6+6)
UnicodeDecoder-8 40.9MB/s ± 0% 49.5MB/s ± 0% +20.96% (p=0.002 n=6+6)
CodeUnmarshal-8 176MB/s ± 0% 199MB/s ± 0% +12.93% (p=0.002 n=6+6)
Updates #28923.
Change-Id: I7a5e2aef51bd4ddf2004aad24210f6f50e01eaeb
Reviewed-on: https://go-review.googlesource.com/c/go/+/151042
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
In golang.org/cl/145218, a feature was added where the JSON decoder
would keep track of the entire path to a field when reporting an
UnmarshalTypeError.
However, we all failed to check if this affected the benchmarks - myself
included, as a reviewer. Below are the numbers comparing the CL's parent
with itself, once it was merged:
name old time/op new time/op delta
CodeDecoder-8 12.9ms ± 1% 28.2ms ± 2% +119.33% (p=0.002 n=6+6)
name old speed new speed delta
CodeDecoder-8 151MB/s ± 1% 69MB/s ± 3% -54.40% (p=0.002 n=6+6)
name old alloc/op new alloc/op delta
CodeDecoder-8 2.74MB ± 0% 109.39MB ± 0% +3891.83% (p=0.002 n=6+6)
name old allocs/op new allocs/op delta
CodeDecoder-8 77.5k ± 0% 168.5k ± 0% +117.30% (p=0.004 n=6+5)
The reason why the decoder got twice as slow is because it now allocated
~40x as many objects, which puts a lot of pressure on the garbage
collector.
The reason is that the CL concatenated strings every time a nested field
was decoded. In other words, practically every field generated garbage
when decoded. This is hugely wasteful, especially considering that the
vast majority of JSON decoding inputs won't return UnmarshalTypeError.
Instead, use a stack of fields, and make sure to always use the same
backing array, to ensure we only need to grow the slice to the maximum
depth once.
The original CL also introduced a bug. The field stack string wasn't
reset to its original state when reaching "d.opcode == scanEndObject",
so the last field in a decoded struct could leak. For example, an added
test decodes a list of structs, and encoding/json before this CL would
fail:
got: cannot unmarshal string into Go struct field T.Ts.Y.Y.Y of type int
want: cannot unmarshal string into Go struct field T.Ts.Y of type int
To fix that, simply reset the stack after decoding every field, even if
it's the last.
Below is the original performance versus this CL. There's a tiny
performance hit, probably due to the append for every decoded field, but
at least we're back to the usual ~150MB/s.
name old time/op new time/op delta
CodeDecoder-8 12.9ms ± 1% 13.0ms ± 1% +1.25% (p=0.009 n=6+6)
name old speed new speed delta
CodeDecoder-8 151MB/s ± 1% 149MB/s ± 1% -1.24% (p=0.009 n=6+6)
name old alloc/op new alloc/op delta
CodeDecoder-8 2.74MB ± 0% 2.74MB ± 0% +0.00% (p=0.002 n=6+6)
name old allocs/op new allocs/op delta
CodeDecoder-8 77.5k ± 0% 77.5k ± 0% +0.00% (p=0.002 n=6+6)
Finally, make all of these benchmarks report allocs by default. The
decoder ones are pretty sensitive to generated garbage, so ReportAllocs
would have made the performance regression more obvious.
Change-Id: I67b50f86b2e72f55539429450c67bfb1a9464b67
Reviewed-on: https://go-review.googlesource.com/c/go/+/167978
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Most of the decoding time is spent in the first Decode loop, since the
rest of the function only deals with the few remaining bytes. Any
unnecessary work done in that loop body matters tremendously.
One such unnecessary bottleneck was the use of the enc.decodeMap table.
Since enc is a pointer receiver, and the field is used within the
non-inlineable function decode64, the decoder must perform a nil check
at every iteration.
To fix that, move the enc.decodeMap uses to the parent function, where
we can lift the nil check outside the loop. That gives roughly a 15%
speed-up. The function no longer performs decoding per se, so rename it.
While at it, remove the now unnecessary receivers.
An unfortunate side effect of this change is that the loop now contains
eight bounds checks on src instead of just one. However, not having to
slice src plus the nil check removal well outweigh the added cost.
The other piece that made decode64 slow was that it wasn't inlined, and
had multiple branches. Use a simple bitwise-or trick suggested by Roger
Peppe, and collapse the rest of the bitwise logic into a single
expression. Inlinability and the reduced branching give a further 10%
speed-up.
Finally, add these two functions to TestIntendedInlining, since we want
them to stay inlinable.
Apply the same refactor to decode32 for consistency, and to let 32-bit
architectures see a similar performance gain for large inputs.
name old time/op new time/op delta
DecodeString/2-8 47.3ns ± 1% 45.8ns ± 0% -3.28% (p=0.002 n=6+6)
DecodeString/4-8 55.8ns ± 2% 51.5ns ± 0% -7.71% (p=0.004 n=5+6)
DecodeString/8-8 64.9ns ± 0% 61.7ns ± 0% -4.99% (p=0.004 n=5+6)
DecodeString/64-8 238ns ± 0% 198ns ± 0% -16.54% (p=0.002 n=6+6)
DecodeString/8192-8 19.5µs ± 0% 14.6µs ± 0% -24.96% (p=0.004 n=6+5)
name old speed new speed delta
DecodeString/2-8 84.6MB/s ± 1% 87.4MB/s ± 0% +3.38% (p=0.002 n=6+6)
DecodeString/4-8 143MB/s ± 2% 155MB/s ± 0% +8.41% (p=0.004 n=5+6)
DecodeString/8-8 185MB/s ± 0% 195MB/s ± 0% +5.29% (p=0.004 n=5+6)
DecodeString/64-8 369MB/s ± 0% 442MB/s ± 0% +19.78% (p=0.002 n=6+6)
DecodeString/8192-8 560MB/s ± 0% 746MB/s ± 0% +33.27% (p=0.004 n=6+5)
Updates #19636.
Change-Id: Ib839577b0e3f5a2bb201f5cae580c61365d92894
Reviewed-on: https://go-review.googlesource.com/c/go/+/151177
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: roger peppe <rogpeppe@gmail.com>
If a for loop has a simple condition and begins with a simple
"if x { break; }"; we can simply add "!x" to the loop's condition.
While at it, simplify a few assignments to use the common pattern
"x := staticDefault; if cond { x = otherValue(); }".
Finally, simplify a couple of var declarations.
Change-Id: I413982c6abd32905adc85a9a666cb3819139c19f
Reviewed-on: https://go-review.googlesource.com/c/go/+/165342
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
I had been finding these over a year or so, but none were big enough
changes to warrant CLs. They're a handful now, so clean them all up in a
single commit.
The smaller bodies get a bit simpler, but most importantly, the larger
bodies get unindented.
Change-Id: I5707a6fee27d4c9ff9efd3d363af575d7a4bf2aa
Reviewed-on: https://go-review.googlesource.com/c/go/+/165340
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Base64-encoding 32 bytes results in a 44-byte string.
While in general a 44-byte string might decode to 33 bytes,
if you take a 44-byte string that actually only encodes 32 bytes,
and you try to decode it into 32 bytes, that should succeed.
Instead it fails trying to do a useless dst[33:] slice operation.
Delete that slice operation.
Noticed while preparing CL 156322.
Change-Id: I8024bf28a65e2638675b980732b2ff91c66c62cf
Reviewed-on: https://go-review.googlesource.com/c/go/+/164628
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Two additions are faster than two multiplications and one addition. The
code seems simpler to me too, as it's more obvious that we advance two
destination bytes for each source byte.
name old time/op new time/op delta
Encode/256-8 374ns ± 0% 331ns ± 0% -11.44% (p=0.008 n=5+5)
Encode/1024-8 1.47µs ± 0% 1.29µs ± 0% -11.89% (p=0.004 n=6+5)
Encode/4096-8 5.85µs ± 1% 5.15µs ± 0% -11.89% (p=0.004 n=6+5)
Encode/16384-8 23.3µs ± 0% 20.6µs ± 0% -11.68% (p=0.004 n=6+5)
Change-Id: Iabc63616c1d9fded55fa668ff41dd49efeaa2ea4
Reviewed-on: https://go-review.googlesource.com/c/go/+/151198
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: roger peppe <rogpeppe@gmail.com>
When parsing nested object, UnmarshalTypeError does not contain actual
path to nested field in original JSON.
This commit change Field to contain the full path to that field. One
can get the Field name by stripping all the leading path elements.
Fixes#22369
Change-Id: I6969cc08abe8387a351e3fb2944adfaa0dccad2a
Reviewed-on: https://go-review.googlesource.com/c/go/+/145218
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Add documentation that individual Write calls are buffered and
copy documentation from bufio.Writer notifying the user to call
Flush and Error when all writes are complete. Remove reference
to "file" since the implementation is general and allows any
io.Writer.
Fixes#30045
Change-Id: I50165470e548f296494e764707fbabe36c665015
Reviewed-on: https://go-review.googlesource.com/c/160680
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Most of the encoding time is spent in the first Encode loop, since the
rest of the function only deals with the few remaining bytes. Any
unnecessary work done in that loop body matters tremendously.
One such unnecessary bottleneck was the use of the enc.encode table.
Since enc is a pointer receiver, and the field is first used within the
loop, the encoder must perform a nil check at every iteration.
Add a dummy use of the field before the start of the loop, to move the
nil check there. After that line, the compiler now knows that enc can't
be nil, and thus the hot loop is free of nil checks.
name old time/op new time/op delta
EncodeToString-4 14.7µs ± 0% 13.7µs ± 1% -6.53% (p=0.000 n=10+10)
name old speed new speed delta
EncodeToString-4 559MB/s ± 0% 598MB/s ± 1% +6.99% (p=0.000 n=10+10)
Updates #20206.
Change-Id: Icbb523a7bd9e470a8be0a448d1d78ade97ed4ff6
Reviewed-on: https://go-review.googlesource.com/c/151158
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
encoding/base64 already skips \r and \n when decoding, so this package
must only deal with spaces and tabs. Those aren't nearly as common, so
we can add a fast path with bytes.ContainsAny to skip the costly alloc
and filtering code.
name old time/op new time/op delta
Decode-8 279µs ± 0% 259µs ± 1% -7.07% (p=0.002 n=6+6)
name old speed new speed delta
Decode-8 319MB/s ± 0% 343MB/s ± 1% +7.61% (p=0.002 n=6+6)
name old alloc/op new alloc/op delta
Decode-8 164kB ± 0% 74kB ± 0% -54.90% (p=0.002 n=6+6)
name old allocs/op new allocs/op delta
Decode-8 12.0 ± 0% 11.0 ± 0% -8.33% (p=0.002 n=6+6)
Change-Id: Idfca8700c52f46eb70a4a7e0d2db3bf0124e4699
Reviewed-on: https://go-review.googlesource.com/c/155964
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Comparing errors using DeepEqual breaks if frame information
is added as proposed in Issue #29934.
Updates #29934.
Change-Id: Ib430c9ddbe588dd1dd51314c408c74c07285e1ff
Reviewed-on: https://go-review.googlesource.com/c/162179
Run-TryBot: Marcel van Lohuizen <mpvl@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
Reviewed-by: Damien Neil <dneil@google.com>
First, we can lift the enc.decodeMap nil check out of the loop.
Second, we can make it clear to the compiler that 'in := src[0]' doesn't
need a bounds check, by making len(src)==0 a single if check that always
stops the loop. This is by far the largest speed-up.
Third, we can use a dst slice index instead of reslicing dst, which
removes work from the loop body.
While at it, we can merge the two 'switch dlen' pieces of code, which
simplifies the code and doesn't affect performance.
name old time/op new time/op delta
DecodeString-8 80.2µs ± 0% 67.5µs ± 0% -15.81% (p=0.002 n=6+6)
name old speed new speed delta
DecodeString-8 163MB/s ± 0% 194MB/s ± 0% +18.78% (p=0.002 n=6+6)
Change-Id: Iefeaae94c03453f8760452b1da706a77b3522718
Reviewed-on: https://go-review.googlesource.com/c/154422
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Both the encoding/xml and encoding/json packages support custom
marshalers for JSON and XML, as well as the basic encoding.TextMarshaler
and encoding.TextUnmarshaler interfaces, but the docs and examples for
these are missing.
There are docs for how to use encoding.TextMarshaler and
encoding.TextUnmarshaler in encoding/json, but not encoding/xml. There
are no examples for how to use them with either json or xml. This commit
includes docs for encoding/xml and examples for both encoding/json and
encoding/xml.
There is an example using custom marshalers MarshalJSON and
UnmarshalJSON in encoding/json, but not MarshalXML and UnmarshalXML in
encoding/json. These docs are more so necessary for encoding/xml because
the complexities of XML documents is significantly greater than JSON
documents which more often leads to the need for custom marshaling. The
encoding/json package includes an example of how to write a custom
marshaler, and this commit includes the same example for the xml
package.
All examples are mirrored off the existing custom marshaler example in
encoding/json.
Fixes#6859
Change-Id: Ic93abc27c0b4d5e48dea6ede4e20b1bedca4ab39
Reviewed-on: https://go-review.googlesource.com/c/76350
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Change-Id: I7a1046f5e0aedbbdd1106a616de410fe4e0cb7d8
Reviewed-on: https://go-review.googlesource.com/c/92295
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Go documentation style for boolean funcs is to say:
// Foo reports whether ...
func Foo() bool
(rather than "returns true if")
This CL also replaces 4 uses of "iff" with the same "reports whether"
wording, which doesn't lose any meaning, and will prevent people from
sending typo fixes when they don't realize it's "if and only if". In
the past I think we've had the typo CLs updated to just say "reports
whether". So do them all at once.
(Inspired by the addition of another "returns true if" in CL 146938
in fd_plan9.go)
Created with:
$ perl -i -npe 's/returns true if/reports whether/' $(git grep -l "returns true iff" | grep -v vendor)
$ perl -i -npe 's/returns true if/reports whether/' $(git grep -l "returns true if" | grep -v vendor)
Change-Id: Ided502237f5ab0d25cb625dbab12529c361a8b9f
Reviewed-on: https://go-review.googlesource.com/c/147037
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Now that the library allows much larger data, it can kill
machines with less memory.
Fixes#28321
Change-Id: I98e1a5fdf812fd75adfb22bf01542423de405fe2
Reviewed-on: https://go-review.googlesource.com/c/143817
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
A little shift magic makes it easy to adjust the maximum buffer
size on machines with larger integers.
Fixes#27635
Change-Id: I1f26b07a363fbb9730df2377052475fa88bbb781
Reviewed-on: https://go-review.googlesource.com/c/143678
Run-TryBot: Rob Pike <r@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Calling .Interface on a struct field's reflect.Value isn't always safe.
For example, if that field is an unexported anonymous struct.
We only descended into this branch if the struct type had any methods,
so this bug had gone unnoticed for a few release cycles.
Add the check, and add a simple test case.
Fixes#28145.
Change-Id: I02f7e0ab9a4a0c18a5e2164211922fe9c3d30f64
Reviewed-on: https://go-review.googlesource.com/c/141537
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Given a program as follows:
data := []byte(`{"F": {
"a": 2,
"3": 4
}}`)
json.Unmarshal(data, &map[string]map[int]int{})
The JSON package should error, as "a" is not a valid integer. However,
we'd encounter a panic:
panic: JSON decoder out of sync - data changing underfoot?
The reason was that decodeState.object would return a nil error on
encountering the invalid map key string, while saving the key type error
for later. This broke if we were inside another object, as we would
abruptly end parsing the nested object, leaving the decoder in an
unexpected state.
To fix this, simply avoid storing the map element and continue decoding
the object, to leave the decoder state exactly as if we hadn't seen an
invalid key type.
This affected both signed and unsigned integer keys, so fix both and add
two test cases.
Updates #28189.
Change-Id: I8a6204cc3ff9fb04ed769df7a20a824c8b94faff
Reviewed-on: https://go-review.googlesource.com/c/142518
Reviewed-by: Ian Lance Taylor <iant@golang.org>
This change makes stateEndTop use isSpace instead of specifically
recreating the same functionality.
Change-Id: I81f8f51682e46e7f8e2b9fed423a968457200625
Reviewed-on: https://go-review.googlesource.com/c/121797
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
I omitted vendor directories and anything necessary for bootstrapping.
(Tested by bootstrapping with Go 1.4)
Updates #27864
Change-Id: I7d9b68d0372d3a34dee22966cca323513ece7e8a
Reviewed-on: https://go-review.googlesource.com/137856
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Having these panic-like errors used to be ok, since they were used in
the internal decoder state instead of passed around via return
parameters.
Recently, the decoder was rewritten to use explicit error returns
instead. This error is a terrible fit for error returns; a handful of
functions must return an error because of it, and their callers must
check for an error that should never happen.
This is precisely what panics are for, so use them. The test coverage of
the package goes up from 91.3% to 91.6%, and performance is unaffected.
We can also get rid of some unnecessary verbosity in the code.
name old time/op new time/op delta
CodeDecoder-4 27.5ms ± 1% 27.5ms ± 1% ~ (p=0.937 n=6+6)
Change-Id: I01033b3f5b7c0cf0985082fa272754f96bf6353c
Reviewed-on: https://go-review.googlesource.com/134835
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
Run-TryBot: Joe Tsai <thebrokentoaster@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
The overall coverage of the json package goes up from 90.8% to 91.3%.
While at it, apply two minor code simplifications found while inspecting
the HTML coverage report.
Change-Id: I0fba968afeedc813b1385e4bde72d93b878854d7
Reviewed-on: https://go-review.googlesource.com/134735
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Russ Cox
jiahua wang
Keith Randall
korzhao
korzhao
Joe Tsai
Roger Peppe
Yury Smolsky
Filippo Valsorda
Ariel Mashraki
Aman Karmani
John Bampton
Katie Hockman
Emmanuel T Odeke
Meng Zhuo
Josh Bleecher Snyder
Eric Chiang
Kevin Burke
Dmitrii Okunev
Ian Lance Taylor
Roland Bracewell Shoemaker
KimMachineGun
Carlos Alexandro Becker
lujjjh
Sean Liao
zhouzhongyuan
Damien Neil
Robert Griesemer
Martin Möhrmann
Daniel Martí
Alex Gaynor
Kirill Korotaev
Tamás Gulácsi
Tim Cooper
Ziheng Liu
codesoap
Jordan Liggitt
Jon Johnson
Than McIntosh
Martin Garton
kaxapi
Udalov Max
Lorenz Bauer
Sam Whited
Phil Pearl
Eugene Kalinin
Francesco Renzi
Cuong Manh Le
Tom Thorogood
Lucas Bremgartner
William Poussier
Sven Taute
andig
Romain Baugue
sergeilem
Ainar Garipov
Eli Bendersky
itchyny
Eddie Scholtz
Dmitry Vyukov
Dominik Honnef
Leon Klingele
Shulhan
Brad Fitzpatrick
Romain Baugue
Rob Pike
Brian Kessler
Marcel van Lohuizen
Ketan Parmar
GuilhermeCaruso
Mostyn Bramley-Moore
Leigh McCulloch
Santhosh Kumar Tekuri
hearot
Mihai Todor
Gabriel Aszalos
Ian Davis
Ivan Kutuzov
Taesu Pyo