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: inconsistent distinction between omitted, null and non-null values #44023

Closed
epsleq0 opened this issue Jan 30, 2021 · 3 comments
Closed

Comments

@epsleq0
Copy link

@epsleq0 epsleq0 commented Jan 30, 2021

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

$ go version
go version go1.15.7 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
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/xxx/.cache/go-build"
GOENV="/home/xxx/.config/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GOMODCACHE="/home/xxx/go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/home/xxx/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/local/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/local/go/pkg/tool/linux_amd64"
GCCGO="gccgo"
AR="ar"
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-build402757801=/tmp/go-build -gno-record-gcc-switches"

What did you do?

https://play.golang.org/p/04udQzPSPKK

Background: My model contains optional values, e.g. Foo *string.
With a JSON based patch method I want to deal with these three cases:

  1. the JSON does not contain the field foo (omitted) -> the field Foo of the model is not changed.
  2. the JSON contains the field foo, this is null -> the field Foo of the model is changed to null.
  3. the JSON contains the foo field, this is non-zero -> the Foo field of the model is set to the non-zero value.

What did you expect to see?

marshalling, test case: 0, result: true
marshalling, test case: 1, result: true
marshalling, test case: 2, result: true
unmarshalling, test case: 0, result: true
unmarshalling, test case: 1, result: true
unmarshalling, test case: 2, result: true

What did you see instead?

marshalling, test case: 0, result: true
marshalling, test case: 1, result: true
marshalling, test case: 2, result: true
unmarshalling, test case: 0, result: true
unmarshalling, test case: 1, result: false
unmarshalling, test case: 2, result: true

In other words, unmarshalling does not distinguish between omitted and null elements, although we have explicitly specified omitempty. The other direction (marshalling) works perfectly.

Unlike #11939, a basic data type of the builtin package (**string) is affected here.

@networkimprov
Copy link

@networkimprov networkimprov commented Feb 4, 2021

cc @mvdan

@mvdan
Copy link
Member

@mvdan mvdan commented Feb 4, 2021

I think the behavior you observe is correct, as per the docs:

To unmarshal JSON into a pointer, Unmarshal first handles the case of the JSON being the JSON literal null. In that case, Unmarshal sets the pointer to nil. Otherwise, Unmarshal unmarshals the JSON into the value pointed at by the pointer. If the pointer is nil, Unmarshal allocates a new value for it to point to.

Remember that omitempty only applies to marshaling, not unmarshaling.

With a JSON based patch method I want to deal with these three cases:

It seems like you should implement a custom string type which handles the three cases properly via UnmarshalJSON. You can have arbitrary logic that way, really. **string isn't quite going to do what you want as per the docs above, and changing those rules now would likely break many existing programs.

@epsleq0
Copy link
Author

@epsleq0 epsleq0 commented Feb 4, 2021

Thank you for clarifying @mvdan

Why I assumed that omitempty is also used for decoding is in fact the following lines of the documentation:

The "omitempty" option specifies that the field should be omitted from the encoding if the field has an empty value, defined as false, 0, a nil pointer, a nil interface value, and any empty array, slice, map, or string.

Unmarshal uses the inverse of the encodings that Marshal uses, allocating maps, slices, and pointers as necessary, with the following additional rules:

Introducing a new type with UnmarshalJSON leads me directly to the open issues in #11939.

Therefore I have now specified a suitable UnmarshalJSON method on the patch object type T, which (depending on omitempty) takes care of decoding U- *U- or **U-values.

@epsleq0 epsleq0 closed this Feb 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants