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

encoding/json: JSON tags don't handle empty properties, non-standard characters #22518

Open
schani opened this Issue Oct 31, 2017 · 10 comments

Comments

Projects
None yet
10 participants
@schani
Copy link

schani commented Oct 31, 2017

What did you do?

https://play.golang.org/p/RRB1VFNufW

Trying to unmarshal with tags like this:

type Data struct {
	Foo    string `json:"Foo"`
	Empty  string `json:""`
	Quote  string "json:\"\\\""
	Smiley string "json:\"\U0001F610\""
}

What did you expect to see?

{"Foo": "bla", "": "nothing", "\"": "quux", "😐": ":-|"}
{"Foo":"bla","":"nothing","\"":"quux","😐":":-|"}

What did you see instead?

{"Foo": "bla", "": "nothing", "\"": "quux", "😐": ":-|"}
{"Foo":"bla","Empty":"","Quote":"","Smiley":""}

System details

go version go1.9.2 darwin/amd64
GOARCH="amd64"
GOBIN=""
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOOS="darwin"
GOPATH="/Users/schani/go"
GORACE=""
GOROOT="/usr/local/go"
GOTOOLDIR="/usr/local/go/pkg/tool/darwin_amd64"
GCCGO="gccgo"
CC="clang"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/1h/7ghts1ys53xdk8y1czrjzdmw0000gn/T/go-build770874315=/tmp/go-build -gno-record-gcc-switches -fno-common"
CXX="clang++"
CGO_ENABLED="1"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOROOT/bin/go version: go version go1.9.2 darwin/amd64
GOROOT/bin/go tool compile -V: compile version go1.9.2
uname -v: Darwin Kernel Version 16.7.0: Thu Jun 15 17:36:27 PDT 2017; root:xnu-3789.70.16~2/RELEASE_X86_64
ProductName:	Mac OS X
ProductVersion:	10.12.6
BuildVersion:	16G29
lldb --version: lldb-900.0.50.1
  Swift-4.0
gdb --version: GNU gdb (GDB) 7.12.1

@ianlancetaylor ianlancetaylor changed the title JSON tags don't handle empty properties, non-standard characters encoding/json: JSON tags don't handle empty properties, non-standard characters Oct 31, 2017

@ianlancetaylor

This comment has been minimized.

Copy link
Contributor

ianlancetaylor commented Oct 31, 2017

This is because of isValidTag (https://golang.org/src/encoding/json/encode.go#L805). Your tags are not considered to be valid, and are therefore ignored. isValidTag was implemented to fix #1520, but I think that issue was before the standard field tag format.

@ianlancetaylor ianlancetaylor added this to the Go1.11 milestone Oct 31, 2017

@ianlancetaylor

This comment has been minimized.

Copy link
Contributor

ianlancetaylor commented Oct 31, 2017

Do other JSON encoders support field names that are not ordinary identifiers?

@schani

This comment has been minimized.

Copy link
Author

schani commented Nov 1, 2017

@ianlancetaylor Newtonsoft.JSON for .NET handles all those cases.

@ibrt

This comment has been minimized.

Copy link
Contributor

ibrt commented Nov 2, 2017

Might be worth noting that the JSON spec(s) [1] [2] allow for any unicode character in object keys.

JSON.parse (ECMAScript 5.1 and up) also supports that.

@bradfitz bradfitz added the NeedsFix label Jun 13, 2018

@bradfitz bradfitz modified the milestones: Go1.11, Go1.12 Jun 13, 2018

@gopherbot

This comment has been minimized.

Copy link

gopherbot commented Oct 6, 2018

Change https://golang.org/cl/140305 mentions this issue: encoding/json: permit any string as a struct field name

@bontibon

This comment has been minimized.

Copy link
Contributor

bontibon commented Oct 17, 2018

@rsc Should this issue be considered with "JSON sweep", or is it fine as-is? The PR that I submitted can be amended to remove the introduced emptykey option.

@mvdan

This comment has been minimized.

Copy link
Member

mvdan commented Nov 18, 2018

I'd like to hear from the other encoding/json owners before we merge that CL - ping @rsc @dsnet. The proposed change seems fine to me in principle, although I can't think of where I'd take advantage of it myself.

@dsnet

This comment has been minimized.

Copy link
Member

dsnet commented Nov 18, 2018

This change seems too narrow-sighted. An emptykey option permits one currently invalid key among many other keys that are still not allowed. For example, what if I wanted a comma to be in my JSON key? Currently that isn't allowed since commas are used as the delimiter between the different tag arguments. Why is an empty string as a key more special than a key with commas?

A more generalized solution might be to allow a quoted string as the name argument:

Field1 int `json:"\"\",string"` // JSON name is ``
Field2 int `json:"\"foo,bar\",string"` // JSON name is `foo,bar`
Field3 int `json:"\"foo\\\"bar\",string"` // JSON name is `foo"bar`

However, this looks ugly since it requires double escaping.

Personally, I'm okay with the fact that the json package can't emit every possible key via struct tags. Using map[string]interface{} is a possible workaround.

@pwxc

This comment has been minimized.

Copy link

pwxc commented Dec 31, 2018

From #29476 .

I think it's a great pity that we can't use Chinese punctuation as json's key. But in fact, I still have other ways to avoid this situation.

What I encountered was that I had to add to the key, but that didn't work. So I used two < to simulate, but < were automatically converted to \u003e(And I dont know why) . So I can only ask other developers in the group not to use .

@andybons

This comment has been minimized.

Copy link
Member

andybons commented Feb 8, 2019

Marking as NeedsDecision since it's not clear what is the correct solution to this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.