Issue 4519063: code review 4519063: json: don't permit literal U+2028 or U+2029 in strings

Issue 4519063: code review 4519063: json: don't permit literal U+2028 or U+2029 in strings (Closed)

Can't Edit
Can't Publish+Mail
Start Review

Created:
12 years, 11 months ago by bradfitz

Modified:
12 years, 11 months ago

Reviewers:
rsc1, golang-dev

CC:
golang-dev

Visibility:
Public.

Description

json: don't permit literal U+2028 or U+2029 in strings See http://timelessrepo.com/json-isnt-a-javascript-subset

Patch Set 1 #

Patch Set 2 : diff -r 0274c28dfc53 https://go.googlecode.com/hg #

Patch Set 3 : diff -r 0274c28dfc53 https://go.googlecode.com/hg #

Created: 12 years, 11 months ago

Download [raw] [tar.bz2]

		Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+29 lines, -1 line)			Patch
	M	src/pkg/json/decode_test.go	View	1	1 chunk	+15 lines, -0 lines	0 comments	Download
	M	src/pkg/json/encode.go	View	1	1 chunk	+14 lines, -1 line	0 comments	Download

Messages

Total messages: 9

Expand All Messages | Collapse All Messages

bradfitz

Hello golang-dev@googlegroups.com (cc: golang-dev@googlegroups.com), I'd like you to review this change to https://go.googlecode.com/hg

12 years, 11 months ago (2011-05-16 17:20:26 UTC) #1

rsc1

But this is a JSON package, not a JavaScript package. Maybe the test belongs in ...

12 years, 11 months ago (2011-05-16 17:32:15 UTC) #2

bradfitz

On Mon, May 16, 2011 at 10:32 AM, Russ Cox <rsc@google.com> wrote: > But this ...

12 years, 11 months ago (2011-05-16 17:36:41 UTC) #3

rsc1

> JSON is supposed to be a subset of JavaScript. The advertising copy certainly says ...

12 years, 11 months ago (2011-05-16 17:43:49 UTC) #4

bradfitz

On Mon, May 16, 2011 at 10:43 AM, Russ Cox <rsc@google.com> wrote: > > JSON ...

12 years, 11 months ago (2011-05-16 17:56:24 UTC) #5

rsc1

>> > JSON is supposed to be a subset of JavaScript. >> >> The advertising ...

12 years, 11 months ago (2011-05-16 18:02:33 UTC) #6

rsc1

Why did this get abandoned? It seems worth doing, just in json.HTMLEscape.

12 years, 11 months ago (2011-05-16 18:23:07 UTC) #8

bradfitz

12 years, 11 months ago (2011-05-16 18:29:43 UTC) #9

Whoops, meant to hg change -D, not -d.

I was feeling argumentative about doing it by default (in Encode and Compact
for people who implement json.Marshaler) and wanted to see how much I cared
later.  Your "I don't care much what web apps do" comment made me feel I
should just drop it.

json.HTMLEscape is intended for people writing JavaScript into <script> tags
in html bodies, not for responses like:

HTTP/1.0 200 OK
Content-Type: application/json

myCallback({"foo": "bar"})

... I don't think people would use HTMLEscape in that context.  I certainly
never do.  I just JSON encode right to my http.ResponseWriter after I've
written "myCallback(".

Having to do all my JSON encoding into a bytes.Buffer first and then to my
ResponseWriter feels lame and therefore I lost interest.

If anything json.HTMLEscape (a weird name for JSONP too) or
json.JavascriptEscape should be a io.Writer->io.Writer instead of a
bytes.Buffer interface.

On Mon, May 16, 2011 at 11:23 AM, Russ Cox <rsc@google.com> wrote:

> Why did this get abandoned?
> It seems worth doing, just in json.HTMLEscape.
>

Expand All Messages | Collapse All Messages