Skip to content
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

Jump to bottom

encoding/json: Unmarshal case insensitive match can lead to nondeterministic behavior #36932

Closed
justinrixx opened this issue Jan 31, 2020 · 3 comments
Closed

encoding/json: Unmarshal case insensitive match can lead to nondeterministic behavior #36932

justinrixx opened this issue Jan 31, 2020 · 3 comments
Labels
FrozenDueToAge
Milestone
Backlog

Comments

@justinrixx
Copy link

justinrixx commented Jan 31, 2020 •
edited

What version of Go are you using (go version)?

$ go version
go version go1.13.3 darwin/amd64

Does this issue reproduce with the latest release?

not sure. i was able to reproduce on my mac (1.13.3), as well as the playground.

What operating system and processor architecture are you using (go env)?

go env Output
$ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN="/Volumes/Code/go/bin"
GOCACHE="/Users/justinr/Library/Caches/go-build"
GOENV="/Users/justinr/Library/Application Support/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GONOPROXY=""
GONOSUMDB=""
GOOS="darwin"
GOPATH="/Volumes/Code/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/local/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/local/go/pkg/tool/darwin_amd64"
GCCGO="gccgo"
AR="ar"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD=""
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/_m/2v0j34ss2xnbndyltfgzgvpr0000gq/T/go-build229954690=/tmp/go-build -gno-record-gcc-switches -fno-common"
GOROOT/bin/go version: go version go1.13.3 darwin/amd64
GOROOT/bin/go tool compile -V: compile version go1.13.3
uname -v: Darwin Kernel Version 19.3.0: Thu Jan  9 20:58:23 PST 2020; root:xnu-6153.81.5~1/RELEASE_X86_64
ProductName:	Mac OS X
ProductVersion:	10.15.3
BuildVersion:	19D76
lldb --version: lldb-1100.0.30.12
Apple Swift version 5.1.3 (swiftlang-1100.0.282.1 clang-1100.0.33.15)

What did you do?

https://play.golang.org/p/6lgm5E0R0Nz

essentially, test the behavior of unmarshal with case-sensitive keys.

What did you expect to see?

according to the wording in the godoc (https://godoc.org/encoding/json#Unmarshal):

Unmarshal matches incoming object keys to the keys used by Marshal (either the struct field name or its tag), preferring an exact match but also accepting a case-insensitive match.

i would expect the json key that exactly matches the tag (fieldSETid) to be unmarshalled to the struct (in this case, it should be "sdkgjfhdfsgj") in any json key ordering.

What did you see instead?

the last case-insensitive match encountered in the json object is the accepted value. this is what differs this from this issue: #11673

this would be completely acceptable if it was deterministic every time. however, json keys are not deterministic (in fact, my understanding is that golang's own marshal does not guarantee key order). this can lead to non-deterministic differences in unmarshal behavior, and there's not really a workaround other than changing the incoming payload (which may not be possible due to the fact that it may be generated using a system that does not have deterministic key order).
@dmitshur dmitshur changed the title encoding/json unmarshal case insensitive match can lead to nondeterministic behavior encoding/json: Unmarshal case insensitive match can lead to nondeterministic behavior Jan 31, 2020
@mvdan
Copy link
Member

mvdan commented Feb 1, 2020

I think this is a duplicate of #14750; in particular, see my proposed solution at #14750 (comment). That would essentially be amending the docs to reflect the behavior we have had for years.

The alternative would be to change the implementation to reflect the documentation - that is, to give preference to the case sensitive match, no matter the order. However, I worry that that could easily break existing programs in subtle ways.

In any case, I think the discussion should continue in that previous issue.

@justinrixx
Copy link
Author

yeah, i agree that the godoc is inconsistent, but i think this is a bug regardless of what the docs say. imo, the correct bug fix is not changing the docs, it's to change the behavior. i don't think it was ever the intention to cause non-deterministic behavior. in certain scenarios, safely unmarshalling the correct key is not supported, barring writing a custom unmarshaller. if there was a flag or something that you could specify to force the match to be case sensitive, then this would be fine. but as long as order determines which key gets unmarshalled, this is going to be an unsafe and unpredictable operation. i'm fine with moving the discussion over to that issue though. thanks for pointing that out to me.

@justinrixx justinrixx mentioned this issue Feb 4, 2020
Open
@cagedmantis cagedmantis added this to the Backlog milestone Feb 7, 2020
@cagedmantis
Copy link
Contributor

@justinrixx Thank you for submitting this issue. I am going to close it as it seems like a duplicate #14750 (as mentioned earlier in the issue).

@golang golang locked and limited conversation to collaborators Feb 6, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge
Projects
None yet
Milestone
Backlog
Development

No branches or pull requests
4 participants
@cagedmantis @justinrixx @mvdan @gopherbot