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

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

Comments

@twmb
Copy link
Contributor

@twmb 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

This comment has been minimized.

Copy link
Contributor

@josharian josharian commented Jan 22, 2020

cc @mvdan

@josharian josharian added this to the Go1.14 milestone Jan 22, 2020
@robpike

This comment has been minimized.

Copy link
Contributor

@robpike robpike commented Jan 22, 2020

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

This comment has been minimized.

Copy link
Contributor Author

@twmb 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

This comment has been minimized.

Copy link
Contributor

@robpike robpike commented Jan 22, 2020

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

@mvdan

This comment has been minimized.

Copy link
Member

@mvdan 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

This comment has been minimized.

Copy link

@gopherbot gopherbot commented Jan 22, 2020

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

@gopherbot gopherbot closed this in 1319bb9 Jan 29, 2020
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
5 participants
You can’t perform that action at this time.