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

Jump to bottom

encoding/json: backport CL 188717 for 1.13 #34006

Closed
mvdan opened this issue Sep 1, 2019 · 3 comments
Closed

encoding/json: backport CL 188717 for 1.13 #34006

mvdan opened this issue Sep 1, 2019 · 3 comments
Labels
CherryPickApproved Used during the release process for point releases FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone
Go1.13

Comments

@mvdan
Copy link
Member

mvdan commented Sep 1, 2019 •
edited

https://golang.org/cl/188717 removes a piece of godoc added in 1.13, since it was incorrect and misleading.

We should backport this for either the final 1.13 release, or 1.13.1.

Not a release blocker, as this is only a minor doc change.

/cc @andybons @dmitshur
@mvdan mvdan added the NeedsFix The path to resolution is known, but the work has not been done. label Sep 1, 2019
@mvdan mvdan added this to the Go1.13 milestone Sep 1, 2019
@dmitshur dmitshur added the CherryPickApproved Used during the release process for point releases label Sep 1, 2019
@dmitshur
Copy link
Contributor

dmitshur commented Sep 1, 2019 •
edited

Approving because it's a documentation fix.

I've looked at the history of relevant issues/CLs and the fix looks valid to me. Compact does not escape <, >, & characters the way Marshal does. E.g., see here.

@gopherbot
Copy link

Change https://golang.org/cl/192747 mentions this issue: [release-branch.go1.13] encoding/json: revert Compact HTML escaping documentation

@gopherbot
Copy link

Closed by merging 2e65ef6 to release-branch.go1.13.

gopherbot pushed a commit that referenced this issue Sep 2, 2019
@tmthrgd @dmitshur
[release-branch.go1.13] encoding/json: revert Compact HTML escaping d…
2e65ef6 
…ocumentation

This partly reverts CL 173417 as it incorrectly documented that Compact
performed HTML escaping and the output was safe to embed inside HTML
<script> tags. This has never been true.

Although Compact does escape U+2028 and U+2029, it doesn't escape <, >
or &. Compact is thus only performing a subset of HTML escaping and it's
output is not safe to embed inside HTML <script> tags.

A more complete fix would be for Compact to either never perform any
HTML escaping, as it was prior to CL 10883045, or to actually perform
the same HTML escaping as HTMLEscape. Neither change is likely safe
enough for go1.13.

Fixes #34006
Updates #30357

Change-Id: I912f0fe9611097d988048b28228c4a5b985080ba
GitHub-Last-Rev: aebabab
GitHub-Pull-Request: #33427
Reviewed-on: https://go-review.googlesource.com/c/go/+/188717
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
(cherry picked from commit 79669dc)
Reviewed-on: https://go-review.googlesource.com/c/go/+/192747
Reviewed-by: Andrew Bonventre <andybons@golang.org>
Run-TryBot: Andrew Bonventre <andybons@golang.org>
@golang golang locked and limited conversation to collaborators Sep 1, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
CherryPickApproved Used during the release process for point releases FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Milestone
Go1.13
Development

No branches or pull requests
3 participants
@dmitshur @mvdan @gopherbot