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>
Robert Griesemer
Martin Möhrmann
Katie Hockman
Daniel Martí
Ian Lance Taylor
Roland Shoemaker
Alex Gaynor
Kirill Korotaev
Tamás Gulácsi
Tim Cooper
Ziheng Liu
codesoap
Jordan Liggitt
Jon Johnson
Filippo Valsorda
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