commit | b1a48af7e8ee87cc46e1bbb07f81ac4853e0f27b | [log] [tgz] |
---|---|---|
author | Daniel Martí <mvdan@mvdan.cc> | Tue Mar 31 12:20:15 2020 +0100 |
committer | Daniel Martí <mvdan@mvdan.cc> | Fri May 08 13:29:11 2020 +0000 |
tree | 0bf86225c84434471c89085173ddaf5978297beb | |
parent | 98d20fb23551a7ab900fcfe9d25fd9cb6a98a07f [diff] |
encoding/json: properly encode strings with ",string" again 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>
Go is an open source programming language that makes it easy to build simple, reliable, and efficient software.
Gopher image by Renee French, licensed under Creative Commons 3.0 Attributions license.
Our canonical Git repository is located at https://go.googlesource.com/go. There is a mirror of the repository at https://github.com/golang/go.
Unless otherwise noted, the Go source files are distributed under the BSD-style license found in the LICENSE file.
Official binary distributions are available at https://golang.org/dl/.
After downloading a binary release, visit https://golang.org/doc/install or load doc/install.html in your web browser for installation instructions.
If a binary distribution is not available for your combination of operating system and architecture, visit https://golang.org/doc/install/source or load doc/install-source.html in your web browser for source installation instructions.
Go is the work of thousands of contributors. We appreciate your help!
To contribute, please read the contribution guidelines: https://golang.org/doc/contribute.html
Note that the Go project uses the issue tracker for bug reports and proposals only. See https://golang.org/wiki/Questions for a list of places to ask questions about the Go language.