code review 5707054: encoding/json: escape output from Marshalers.

encoding/json: escape output from Marshalers.

Fixes issue 3127.

[dsymonds, 2012-02-29 00:25:01]

Hello rsc (cc: golang-dev@googlegroups.com),

I'd like you to review this change to
https://go.googlecode.com/hg/

[dsymonds, 2012-02-29 00:25:48]

Suggestions on making it more efficient are welcome, short of tearing
apart the encoding machinery. Someone else can tackle that later.


Dave.

[r, 2012-02-29 00:34:49]

http://codereview.appspot.com/5707054/diff/2001/src/pkg/encoding/json/encode.go
File src/pkg/encoding/json/encode.go (right):

http://codereview.appspot.com/5707054/diff/2001/src/pkg/encoding/json/encode....
src/pkg/encoding/json/encode.go:269: var buf bytes.Buffer
is it worth a call to bytes.IndexAny to avoid the allocation? i think so.

[dsymonds, 2012-02-29 00:53:39]

http://codereview.appspot.com/5707054/diff/2001/src/pkg/encoding/json/encode.go
File src/pkg/encoding/json/encode.go (right):

http://codereview.appspot.com/5707054/diff/2001/src/pkg/encoding/json/encode....
src/pkg/encoding/json/encode.go:269: var buf bytes.Buffer
On 2012/02/29 00:34:49, r wrote:
> is it worth a call to bytes.IndexAny to avoid the allocation? i think so.

It would improve this particular approach, yeah. I want to see whether Russ
wants me to take a much more different approach (e.g. slotting an escaping
io.Writer in somewhere).

[rsc, 2012-02-29 15:50:20]

http://codereview.appspot.com/5707054/diff/2001/src/pkg/encoding/json/encode.go
File src/pkg/encoding/json/encode.go (right):

http://codereview.appspot.com/5707054/diff/2001/src/pkg/encoding/json/encode....
src/pkg/encoding/json/encode.go:269: var buf bytes.Buffer
On 2012/02/29 00:53:39, dsymonds wrote:
> On 2012/02/29 00:34:49, r wrote:
> > is it worth a call to bytes.IndexAny to avoid the allocation? i think so.
> 
> It would improve this particular approach, yeah. I want to see whether Russ
> wants me to take a much more different approach (e.g. slotting an escaping
> io.Writer in somewhere).

I don't want to slow things down noticeably here, but since we're
already checking validity and compacting below, it should be 
both easy and quick to make that processing do the escaping too.
A proposal:

Rename Compact to compact with an extra parameter
and a little bit more code:

func compact(dst *bytes.Buffer, src []byte, escape bool) error {
	...
	for i, c := range src {
		if escape && (c == '<' || c == '>' || c == '&') {
			if start < i {
				dst.Write(src[start:i])
			}
			dst.WriteString(`\u00`)
			dst.WriteByte(hex[c>>4])
			dst.WriteByte(hex[c&0xF])
			start = i+1
		}
		v := scan.step(&scan, int(c))
		...
		
Then Compact == compact(dst, src, false), and below the 
call can be compact(&e.Buffer, b, true).

[dsymonds, 2012-03-01 05:42:34]

Hello rsc@golang.org, r@golang.org (cc: golang-dev@googlegroups.com),

Please take another look.

[rsc, 2012-03-01 06:26:53]

LGTM

[dsymonds, 2012-03-01 06:42:05]

*** Submitted as http://code.google.com/p/go/source/detail?r=fe8fee76a008 ***

encoding/json: escape output from Marshalers.

Fixes issue 3127.

R=rsc, r
CC=golang-dev
http://codereview.appspot.com/5707054