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

net: IPNet JSON marshal/unmarshal inconsistency #35727

Open
elad opened this issue Nov 21, 2019 · 3 comments
Milestone

Comments

@elad
Copy link

@elad elad commented Nov 21, 2019

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

$ go version
go version go1.13.4 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
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/elad/Library/Caches/go-build"
GOENV="/Users/elad/Library/Application Support/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOOS="darwin"
GOPATH="/Users/elad/go"
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/local/Cellar/go/1.13.4/libexec"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/local/Cellar/go/1.13.4/libexec/pkg/tool/darwin_amd64"
GCCGO="gccgo"
AR="ar"
CC="clang"
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"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/12/1cmrxkzd3mjdw85xz7jj36cr0000gn/T/go-build857110316=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

Wrote code that uses net.ParseCIDR() to create a *net.IPNet, then uses json.Marshal() to serialize it to JSON, then uses json.Unmarshal() to deserialize it back to *net.IPNet.

Here's a link: https://play.golang.org/p/ryc7rp4KyiZ

Code just in case:

package main

import (
	"encoding/json"
	"net"
	"testing"
	
	"github.com/stretchr/testify/assert"
)

func TestMarshal(t *testing.T) {
	var err error

	_, cidrOut, err := net.ParseCIDR("192.168.0.0/16")
	assert.Nil(t, err, "net.ParseCIDR")

	cidrJson, err := json.Marshal(cidrOut)
	assert.Nil(t, err, "json.Marshal")

	var cidrIn *net.IPNet
	err = json.Unmarshal(cidrJson, &cidrIn)
	assert.Nil(t, err, "json.Unmarshal")
	
	assert.Equal(t, cidrOut, cidrIn, "CIDR value differs")
	assert.Equal(t, len(cidrOut.IP), len(cidrIn.IP), "IP length differs")
}

What did you expect to see?

I expected that the original and deserialized values will be equal.

What did you see instead?

I saw that the original and deserialized values are different.

Specifically, I saw that the original value as returned from net.ParseCIDR() uses 4-byte representation and the value as deserialized by json.Unmarshal() uses 16-byte representation. The values might be semantically equivalent but the internal byte representation is different. Note that this value (net.IPNet.IP) is managed internally and not used at all by the calling code.

I think this might be because net.ParseCIDR() masks the net.IPNet.IP field with a mask that is limited by the IP address length and by default assumes IPv4, so 4-byte representation. The result is that for an IPv4 CIDR, the internal net.IPNet.IP will always be in 4-byte representation. The IP.UnmarshalText() function uses net.ParseIP(), though, which calls parseIPv4(), which in turn uses net.IPv4() to construct the net.IP that is returned. But net.IPv4() uses 16-byte representation ("v4-in-v6 prefix").

I think this is not ideal since in order to get semantics where input is equal to output one has to add code as below, which alters internal data structures:

v4 := cidr.IP.To4(); if v4 != nil {
    cidr.IP = v4
}

If this is indeed considered a problem, a naive suggestion would be to add an unmarshal function for net.IPNet that uses net.ParseCIDR() and thus retaining semantics. I don't know if that's viable though or what unforeseen effects this might have.

@wI2L

This comment has been minimized.

Copy link
Contributor

@wI2L wI2L commented Nov 21, 2019

/cc @mvdan

@mvdan mvdan changed the title net.IPNet JSON marshal/unmarshal inconsistency net: IPNet JSON marshal/unmarshal inconsistency Nov 21, 2019
@toothrot toothrot added this to the Backlog milestone Nov 25, 2019
@toothrot

This comment has been minimized.

Copy link
Contributor

@toothrot toothrot commented Nov 25, 2019

Also /cc @mikioh

@elad

This comment has been minimized.

Copy link
Author

@elad elad commented Nov 25, 2019

Another way to reproduce the problem, without *net.IPNet, is to call IP.To4() and then do the serialization and deserialization.

Playground: https://play.golang.org/p/VgWt3PtRN8o

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