encoding/json: incorrect object key unmarshaling when using custom TextUnmarshaler as Key with string values #39555
Comments
toothrot
added
the
NeedsInvestigation
|
/cc @rsc |
|
cc @mvdan |
|
Thank you for the report. This is indeed a different regression caused by the same original performance optimization, https://golang.org/cl/190659. The previous fix was good as far as I can tell, but it evidently wasn't a complete fix. It's pretty obvious that the original optimization was a mistake, and applying more best-effort fixes seems too little and too late at this point. I'll send a CL to revert the entire thing, and add more tests. We can backport this into 1.14.5. @gopherbot please consider this for backport to 1.14, it's a regression. |
|
Backport issue(s) opened: #39585 (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. |
mvdan
added
NeedsFix
|
Change https://golang.org/cl/237838 mentions this issue: |
|
Change https://golang.org/cl/241081 mentions this issue: |
…ing strings, take 2" 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. For #39555. Fixes #39585. Change-Id: I66a6919c2dd6d9789232482ba6cf3814eaa70f61 Reviewed-on: https://go-review.googlesource.com/c/go/+/237838 Run-TryBot: Daniel Martí <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> Reviewed-by: Andrew Bonventre <[email protected]> (cherry picked from commit 11389ba) Reviewed-on: https://go-review.googlesource.com/c/go/+/241081 Reviewed-by: Daniel Martí <[email protected]> Run-TryBot: Dmitri Shuralyov <[email protected]>
What version of Go are you using (
go version)?Does this issue reproduce with the latest release?
Yes.
What operating system and processor architecture are you using (
go env)?go envOutputWhat did you do?
I have a custom type with
MarshalTextandUnmarshalTextmethods. I use it as a key in a map, and the value of the map isstring(not reproducible withstruct{}), and Ijson.Marshalthat map and thenjson.Unmarshalit again.https://play.golang.org/p/D1_ncEiDYT-
What did you expect to see?
UnmarshalTextshould receive the data that was returned byMarshalText.What did you see instead?
The data passed to
UnmarshalTextis an escaped version of the data returned byMarshalText, for example"turns into\"and\turns into\\.At first I thought I was running into #38105, but the issue still exists in Go 1.14.4.
The text was updated successfully, but these errors were encountered: