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: wrong encoding for json.Number field with string option (struct tag) #34268

Closed
breml opened this issue Sep 12, 2019 · 3 comments
Closed

encoding/json: wrong encoding for json.Number field with string option (struct tag) #34268

breml opened this issue Sep 12, 2019 · 3 comments
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone
Go1.14

Comments

@breml
Copy link
Contributor

breml commented Sep 12, 2019

If the string option is set on a struct field of type json.Number, the value gets encoded without quotes. If this encoded value is then unmarshaled back, it fails, because the expected quotes are missing.

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

$ go version
go version go1.12.5 linux/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="/home/lubr/.cache/go-build"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="/home/lubr/go"
GOPROXY=""
GORACE=""
GOROOT="/home/lubr/.gvm/versions/go1.12.5.linux.amd64"
GOTMPDIR=""
GOTOOLDIR="/home/lubr/.gvm/versions/go1.12.5.linux.amd64/pkg/tool/linux_amd64"
GCCGO="gccgo"
CC="gcc"
CXX="g++"
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 -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build368255501=/tmp/go-build -gno-record-gcc-switches"

What did you do?

package main

import (
	"encoding/json"
	"fmt"
	"log"
)

type stringTest struct {
	JSONNumberStringOption json.Number `json:",string"`
}

func main() {
	b, err := json.Marshal(stringTest{
		JSONNumberStringOption: "2",
	})
	if err != nil {
		log.Fatal(err)
	}
	fmt.Println(string(b))

	var res stringTest
	err = json.Unmarshal(b, &res)
	if err != nil {
		log.Fatal(err)
	}
}

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

What did you expect to see?

{"JSONNumberStringOption":"2"}

What did you see instead?

{"JSONNumberStringOption":2}
2019/09/12 21:33:02 json: invalid use of ,string struct tag, trying to unmarshal unquoted value into json.Number
exit status 1
breml added a commit to breml/go that referenced this issue Sep 12, 2019
@breml
encoding/json: fix marshal for json.Number ,string
dde81b7 
Add quotes if a value of type json.Number with the string option
(struct tag) set is marshaled. This ensures, that the resulting json
can be unmarshaled into the source struct without error.

Fixes golang#34268
@breml breml mentioned this issue Sep 12, 2019
Closed
@gopherbot
Copy link

Change https://golang.org/cl/195043 mentions this issue: encoding/json: fix marshal for json.Number ,string

@mvdan
Copy link
Member

mvdan commented Sep 13, 2019

The fact that you can encode but then not decode back seems to point clearly at a bug here; thanks for spotting this.

@mvdan mvdan added the NeedsFix The path to resolution is known, but the work has not been done. label Sep 13, 2019
@mvdan mvdan added this to the Go1.14 milestone Sep 13, 2019
@mdempsky
Copy link
Member

I added and then quickly deleted this comment, but I think a few people still saw it via email notifications, so I figured I might as well restore it for posterity:

Package json's documentation describes:

The "string" option signals that a field is stored as JSON inside a JSON-encoded string. It applies only to fields of string, floating point, integer, or boolean types.

So using it for json.Number is technically against the documentation.

I don't have any preference here; but if json.Number is updated to support the "string" option, then this documentation should also be updated.

I made this comment before realizing that Number is a declared type whose underlying type is string. After realizing that, however, I didn't feel confident about my interpretation and wanted to retract it, rather than add uninformed drive-by chatter. (But you can't delete anything from the Internet.)

@golang golang locked and limited conversation to collaborators Sep 15, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Milestone
Go1.14
Development

Successfully merging a pull request may close this issue.

encoding/json: make Number with the ,string option marshal with quotes breml/go
4 participants
@mdempsky @mvdan @breml @gopherbot