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.

Sign up for GitHub

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: behavior change for go1.14 for paragraph-sep and line-sep characters #36690

Closed
twmb opened this issue Jan 22, 2020 · 6 comments
Closed

encoding/json: behavior change for go1.14 for paragraph-sep and line-sep characters #36690

twmb opened this issue Jan 22, 2020 · 6 comments
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone
Go1.14

Comments

@twmb
Copy link
Contributor

twmb commented Jan 22, 2020

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

$ go version
go version devel +71bbffbc48 Wed Jan 22 00:45:41 2020 +0000 linux/amd64

Does this issue reproduce with the latest release?

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

go env Output
$ go env
GO111MODULE="on"
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/twmb/.cache/go-build"
GOENV="/home/twmb/.config/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GOOS="linux"
GOPATH="/home/twmb/go"
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/home/twmb/go/go.src"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/home/twmb/go/go.src/pkg/tool/linux_amd64"
GCCGO="gccgo"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/dev/null"
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-build643685161=/tmp/go-build -gno-record-gcc-switches"
GOROOT/bin/go version: go version devel +71bbffbc48 Wed Jan 22 00:45:41 2020 +0000 linux/amd64
GOROOT/bin/go tool compile -V: compile version devel +71bbffbc48 Wed Jan 22 00:45:41 2020 +0000
uname -sr: Linux 4.15.0-74-generic
Distributor ID:	Ubuntu
Description:	Ubuntu 18.04.3 LTS
Release:	18.04
Codename:	bionic
gdb --version: GNU gdb (Ubuntu 8.1-0ubuntu3.2) 8.1.0.20180409-git

What did you do?

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

What did you expect to see?

<nil>
[34 92 117 50 48 50 56 92 117 50 48 50 57 34]

What did you see instead?

<nil>
[34 226 128 168 226 128 169 34]
@josharian
Copy link
Contributor

cc @mvdan

@josharian josharian added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Jan 22, 2020
@josharian josharian added this to the Go1.14 milestone Jan 22, 2020
@robpike
Copy link
Contributor

robpike commented Jan 22, 2020 •
edited

This is a behavior change, but I didn't dig into how far back. Here is an easier way to see what's happening (https://play.golang.org/p/c-EBn0iL9DD):

% go run x.go
[ 22  2028  2029  22]
<nil>
`"

"`, [22 2028 2029 22]
% go1.4 run x.go
[ 22  2028  2029  22]
<nil>
`"\u2028\u2029"`, [22 5c 75 32 30 32 38 5c 75 32 30 32 39 22]
%

This change was introduced here: 900ebcf

I can't speak to its merits, but it's deliberate.

@twmb
Copy link
Contributor Author

twmb commented Jan 22, 2020

FWIW, I prefer the new behavior. It was weird to me that Compact actually could grow the input text.

As is now, there is no option to just return just JSONP valid JSON. The new defaults for the json package are either complete HTML escaping or no HTML escaping, with both Marshal and NewEncoder defaulting to complete HTML escaping.

IMO that's fine and this issue can be closed or transitioned into a "document the behavior change" issue.

@robpike
Copy link
Contributor

robpike commented Jan 22, 2020

It is a behavior change that will need to be listed in the release notes.

@mvdan
Copy link
Member

mvdan commented Jan 22, 2020

The documentation wasn't touched in the CL because the docs never made any mention of character escaping. The new implementation matches the docs more closely.

I agree that this should be in the release notes, though. Sending a CL.

@gopherbot
Copy link

Change https://golang.org/cl/215817 mentions this issue: doc: add the change to json.Compact in the 1.14 changelog

@golang golang locked and limited conversation to collaborators Jan 28, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Milestone
Go1.14
Development

No branches or pull requests
5 participants
@josharian @twmb @mvdan @robpike @gopherbot