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
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.

Comments

@epsleq0
Copy link

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.

@seankhliao seankhliao added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Jan 30, 2021
@networkimprov
Copy link

cc @mvdan

@mvdan
Copy link
Member

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 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 as completed Feb 4, 2021
@golang golang locked and limited conversation to collaborators Feb 4, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Development

No branches or pull requests

5 participants