Issue 10883045: code review 10883045: encoding/json: escape U+2028 and U+2029.

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+60 lines, -6 lines)			Patch
M	src/pkg/encoding/json/decode_test.go	View	1	1 chunk	+3 lines, -3 lines	0 comments	Download
M	src/pkg/encoding/json/encode.go	View	1 2 3 4 5 6	3 chunks	+29 lines, -3 lines	0 comments	Download
M	src/pkg/encoding/json/indent.go	View	1 2 3 4 5 6	1 chunk	+9 lines, -0 lines	0 comments	Download
M	src/pkg/encoding/json/scanner_test.go	View	1 2 3 4 5	1 chunk	+19 lines, -0 lines	0 comments	Download

Messages

Total messages: 18

Expand All Messages | Collapse All Messages

dsymonds

Hello golang-dev@googlegroups.com, I'd like you to review this change to https://code.google.com/p/go

10 years, 9 months ago (2013-07-04 06:17:16 UTC) #1

bradfitz

Last time I sent a patch for this, rsc rejected it. On Wed, Jul 3, ...

10 years, 9 months ago (2013-07-04 19:57:00 UTC) #2

Last time I sent a patch for this, rsc rejected it.



On Wed, Jul 3, 2013 at 11:17 PM, <dsymonds@golang.org> wrote:

> Reviewers: golang-dev1,
>
> Message:
> Hello golang-dev@googlegroups.com,
>
> I'd like you to review this change to
> https://code.google.com/p/go
>
>
> Description:
> encoding/json: escape U+2028 and U+2029.
>
> Fixes issue 5836.
>
> Please review this at
https://codereview.appspot.**com/10883045/<https://codereview.appspot.com/108...
>
> Affected files:
>   M src/pkg/encoding/json/decode_**test.go
>   M src/pkg/encoding/json/encode.**go
>
>
> Index: src/pkg/encoding/json/decode_**test.go
> ==============================**==============================**=======
> --- a/src/pkg/encoding/json/**decode_test.go
> +++ b/src/pkg/encoding/json/**decode_test.go
> @@ -568,14 +568,14 @@
>  }
>
>  func TestEscape(t *testing.T) {
> -       const input = `"foobar"<html>`
> -       const expected = `"\"foobar\"\u003chtml\u003e"`
> +       const input = `"foobar"<html>` + " [\u2028 \u2029]"
> +       const expected = `"\"foobar\"\u003chtml\u003e [\u2028 \u2029]"`
>         b, err := Marshal(input)
>         if err != nil {
>                 t.Fatalf("Marshal error: %v", err)
>         }
>         if s := string(b); s != expected {
> -               t.Errorf("Encoding of [%s] was [%s], want [%s]", input, s,
> expected)
> +               t.Errorf("Encoding of [%s]:\n got [%s]\nwant [%s]", input,
> s, expected)
>         }
>  }
>
> Index: src/pkg/encoding/json/encode.**go
> ==============================**==============================**=======
> --- a/src/pkg/encoding/json/**encode.go
> +++ b/src/pkg/encoding/json/**encode.go
> @@ -548,6 +548,14 @@
>                 if c == utf8.RuneError && size == 1 {
>                         e.error(&InvalidUTF8Error{s})
>                 }
> +               if c == '\u2028' || c == '\u2029' {
> +                       if start < i {
> +                               e.WriteString(s[start:i])
> +                       }
> +                       e.WriteString(`\u202`)
> +                       e.WriteByte(hex[c&0xF])
> +                       start = i + size
> +               }
>                 i += size
>         }
>         if start < len(s) {
>
>
> --
>
> ---You received this message because you are subscribed to the Google
> Groups "golang-dev" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to
golang-dev+unsubscribe@**googlegroups.com<golang-dev%2Bunsubscribe@googlegrou...
> .
> For more options, visit
https://groups.google.com/**groups/opt_out<https://groups.google.com/groups/o...
> .
>
>
>

dsymonds

On 5 July 2013 05:56, Brad Fitzpatrick <bradfitz@golang.org> wrote: > Last time I sent a ...

10 years, 9 months ago (2013-07-04 23:47:28 UTC) #3

bradfitz

I agree with you. On Jul 4, 2013 4:47 PM, "David Symonds" <dsymonds@golang.org> wrote: > ...

10 years, 9 months ago (2013-07-04 23:50:10 UTC) #4

https://codereview.appspot.com/10883045/diff/6001/src/pkg/encoding/json/encode.go File src/pkg/encoding/json/encode.go (right): https://codereview.appspot.com/10883045/diff/6001/src/pkg/encoding/json/encode.go#newcode551 src/pkg/encoding/json/encode.go:551: if c == '\u2028' || c == '\u2029' { ...

10 years, 9 months ago (2013-07-05 00:04:17 UTC) #5

dsymonds

10 years, 9 months ago (2013-07-05 00:28:51 UTC) #6

rsc

NOT LGTM Same comment I made on Brad's CL: it seems like this belongs in ...

10 years, 9 months ago (2013-07-08 19:27:30 UTC) #8

dsymonds

The input to this is not HTML, nor is the output, nor is the output ...

10 years, 9 months ago (2013-07-09 00:08:33 UTC) #9

rsc

OK but this CL is incomplete. HTMLEscape and func compact should both do the conversion ...

10 years, 9 months ago (2013-07-12 02:11:18 UTC) #11

dsymonds

On 12 July 2013 12:11, <rsc@golang.org> wrote: > func compact should do this too. compact ...

10 years, 9 months ago (2013-07-12 04:03:26 UTC) #13

rsc

I don't see any change to isSpace, which is good. U+2028 or U+2029 outside quoted ...

10 years, 9 months ago (2013-07-12 04:09:19 UTC) #14

dsymonds

On 12 July 2013 14:09, <rsc@golang.org> wrote: > I don't see any change to isSpace, ...

10 years, 9 months ago (2013-07-12 04:10:02 UTC) #15

rsc

LGTM https://codereview.appspot.com/10883045/diff/32001/src/pkg/encoding/json/encode.go File src/pkg/encoding/json/encode.go (right): https://codereview.appspot.com/10883045/diff/32001/src/pkg/encoding/json/encode.go#newcode160 src/pkg/encoding/json/encode.go:160: // so just scan the string one rune ...

10 years, 9 months ago (2013-07-12 04:31:35 UTC) #17

dsymonds

10 years, 9 months ago (2013-07-12 04:36:05 UTC) #18

*** Submitted as https://code.google.com/p/go/source/detail?r=7e8952db0133 ***

encoding/json: escape U+2028 and U+2029.

Fixes issue 5836.

R=golang-dev, bradfitz, r, rsc
CC=golang-dev
https://codereview.appspot.com/10883045

https://codereview.appspot.com/10883045/diff/32001/src/pkg/encoding/json/enco...
File src/pkg/encoding/json/encode.go (right):

https://codereview.appspot.com/10883045/diff/32001/src/pkg/encoding/json/enco...
src/pkg/encoding/json/encode.go:160: // so just scan the string one rune at a
time.
On 2013/07/12 04:31:35, rsc wrote:
> s/rune/byte/

Done.

https://codereview.appspot.com/10883045/diff/32001/src/pkg/encoding/json/enco...
src/pkg/encoding/json/encode.go:177: dst.WriteString(`\u20`)
On 2013/07/12 04:31:35, rsc wrote:
> dst.WriteString(`\u202`)
> dst.WriteByte(hex[src[i+2]&0xF])
> start = i + 3

Done.

https://codereview.appspot.com/10883045/diff/32001/src/pkg/encoding/json/inde...
File src/pkg/encoding/json/indent.go (right):

https://codereview.appspot.com/10883045/diff/32001/src/pkg/encoding/json/inde...
src/pkg/encoding/json/indent.go:35: dst.WriteString(`\u20`)
On 2013/07/12 04:31:35, rsc wrote:
> dst.WriteString(`\u202`)
> dst.WriteByte(hex[src[i+2]&0xF])
> start = i + 3

Done.

Expand All Messages | Collapse All Messages