Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.
Sign upencoding/json: mangled unmarshal string result [1.14 backport] #38106
Labels
Milestone
Comments
|
The CL fixing this issue has been available since March - it's proving difficult to get the change properly approved during this time, so any help is appreciated. It would be a shame for it to get pushed back to 1.14.4 once again. |
|
Approved as this is a Go 1.14-specific regression with no workaround. |
|
Change https://golang.org/cl/233057 mentions this issue: |
|
Closed by merging 846c00e to release-branch.go1.14. |
gopherbot
pushed a commit
that referenced
this issue
May 27, 2020
…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í <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
(cherry picked from commit 55361a2)
Reviewed-on: https://go-review.googlesource.com/c/go/+/233057
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
@dsnet requested issue #38105 to be considered for backport to the next 1.14 minor release.