-
Notifications
You must be signed in to change notification settings - Fork 17.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
encoding/json: mangled unmarshal string result #38105
Comments
@gopherbot, please open a backport issue for 1.14. |
Backport issue(s) opened: #38106 (for 1.14). Remember to create the cherry-pick CL(s) as soon as the patch is submitted to master, according to https://golang.org/wiki/MinorReleases. |
Thanks @dsnet - I'll try to have a look at it this weekend. |
Change https://golang.org/cl/226218 mentions this issue: |
Change https://golang.org/cl/233057 mentions this issue: |
…e case when decoding 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 #38106. For #38105. For #38126. Change-Id: I761e54924e9a971a4f9eaa70bbf72014bb1476e6 Reviewed-on: https://go-review.googlesource.com/c/go/+/226218 Run-TryBot: Daniel Martí <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> Reviewed-by: Joe Tsai <[email protected]> (cherry picked from commit 55361a2) Reviewed-on: https://go-review.googlesource.com/c/go/+/233057 Run-TryBot: Dmitri Shuralyov <[email protected]> Reviewed-by: Daniel Martí <[email protected]>
What version of Go are you using (
go version
)?Does this issue reproduce with the latest release?
Yes, it reproduces with the latest release.
What operating system and processor architecture are you using (
go env
)?go env
OutputWhat did you do?
I create a type whose real type is string and implement
encoding.TextUnmarshaler
andencoding.TextUnmarshaler
. I use this type as a map's key type. Then I just simply do a roundtrip test withencoding/json
'sMarshal
andUnmarshal
function. When my input json contains non-ascii characters, something unexpected happened.What did you expect to see?
What did you see instead?
The text was updated successfully, but these errors were encountered: