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: accept float starting with "." (".85") while unmarshaling json with ,string tag #32117

Open
ashwin1dd opened this issue May 17, 2019 · 6 comments
Open

encoding/json: accept float starting with "." (".85") while unmarshaling json with ,string tag #32117

ashwin1dd opened this issue May 17, 2019 · 6 comments
Labels
NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made.
Milestone
Backlog

Comments

@ashwin1dd
Copy link

ashwin1dd commented May 17, 2019 •
edited

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

❯ go version
go version go1.12.5 darwin/amd64

Does this issue reproduce with the latest release?

yes

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

go env Output
$ go env
GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/ashwin/Library/Caches/go-build"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOOS="darwin"
GOPATH="/Users/ashwin/go"
GOPROXY=""
GORACE=""
GOROOT="/usr/local/Cellar/go/1.12.5/libexec"
GOTMPDIR=""
GOTOOLDIR="/usr/local/Cellar/go/1.12.5/libexec/pkg/tool/darwin_amd64"
GCCGO="gccgo"
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/nl/_f7rxwqs2y730r18nk0shs240000gn/T/go-build739953451=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

Tried unmarshaling json that contained float string starting with "."
The playground link below contains 3 examples

  1. json unmarshal with ".85" -- fails
  2. json unmarshal with "0.85" -- succeeds
  3. strconv.ParseFloat ".85" -- succeeds

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

What did you expect to see?

I expect ".85" to be successfully converted to a float using unmarshal
".85" -> 0.85

What did you see instead?

this error: json: invalid use of ,string struct tag, trying to unmarshal ".85" into float64
@josharian
Copy link
Contributor

That’s not valid json. See the number flow chart at http://json.org/

@bradfitz
Copy link
Contributor

But I think this is specifically about the ,string modifier in our JSON package. We could make it work if we wanted, without violating the JSON grammar.

@bradfitz bradfitz changed the title Accept float starting with "." (".85") while unmarshaling json with ,string tag encoding/json: accept float starting with "." (".85") while unmarshaling json with ,string tag May 18, 2019
@bradfitz bradfitz added the NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. label May 18, 2019
@bradfitz bradfitz added this to the Go1.14 milestone May 18, 2019
@jamdagni86
Copy link
Contributor

@bradfitz / @josharian does this need to be fixed?

@josharian
Copy link
Contributor

It is waiting for a decision to be made. That might take a while. There definitely won’t be any code changes for it in the next month and a half or so.

Also, cc @mvdan

@rsc rsc modified the milestones: Go1.14, Backlog Oct 9, 2019
@dsnet
Copy link
Member

dsnet commented Nov 11, 2020

Even with the ,string modifier, I believe that it should still be parsed according to the JSON number syntax. Users depend on the semantics being well defined so that they can concretely explain to end-users what their services accepts as valid input.

It is simple for users to explain:

This field accepts a JSON number or a JSON string containing a JSON number.

Rather than explaining something more obscure like:

This field accepts a JSON number or a JSON string containing a number that is valid according to the Go grammar for a number.

One of the purposes of JSON is that it's a language agnostic data-interchange format and I think it's a mistake to leak Go-specific details in our implementation. It should theoretically be possible to rewrite a Go service using encoding/json into another language like C++, Rust, Python, etc. and not force the port to implement Go-specific details like how we parse numbers.

@mvdan
Copy link
Member

mvdan commented Nov 24, 2020

I strongly agree with @dsnet that we should reject this. If someone really wants to implement a more magic integer-inside-string type, they always can do it with the Marshaler/Unmarshaler interfaces.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made.
Projects
None yet
Milestone
Backlog
Development

No branches or pull requests
7 participants
@bradfitz @josharian @rsc @jamdagni86 @mvdan @dsnet @ashwin1dd