code review 1496042: json: Add HTMLEscape

json: Add HTMLEscape

[mstetson, 2010-06-03 05:27:12]

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

I'd like you to review this change.

[rsc, 2010-06-03 05:44:08]

The signature should probably be similar to
Compact and Indent, for consistency.

func HTMLEscape(dst *bytes.Buffer, src []byte)

Having a destination buffer also avoids having to
explain when the returned byte slice is shared,
when it's not, etc.

Also it's probably worth avoiding the individual
calls to WriteByte for every single byte.  Instead
keep track of the index of the first byte that needs
to be written and write bigger slices.  See the
implementation of Compact.

Russ

On Wed, Jun 2, 2010 at 22:27,  <micah.stetson@gmail.com> wrote:
> Reviewers: rsc,
>
> Message:
> Hello rsc (cc: golang-dev@googlegroups.com),
>
> I'd like you to review this change.
>
>
> Description:
> json: Add HTMLEscape
>
> Please review this at http://codereview.appspot.com/1496042/show
>
> Affected files:
>  M src/pkg/json/decode_test.go
>  M src/pkg/json/encode.go
>
>
> Index: src/pkg/json/decode_test.go
> ===================================================================
> --- a/src/pkg/json/decode_test.go
> +++ b/src/pkg/json/decode_test.go
> @@ -139,6 +139,13 @@
>        }
>  }
>
> +func TestHTMLEscape(t *testing.T) {
> +       b := HTMLEscape([]byte(`"<>&"`))
> +       if !bytes.Equal(b, []byte(`"\u003c\u003e\u0026"`)) {
> +               t.Errorf("Unexpected encoding of \"<>&\": %s", b)
> +       }
> +}
> +
>  func noSpace(c int) int {
>        if isSpace(c) {
>                return -1
> Index: src/pkg/json/encode.go
> ===================================================================
> --- a/src/pkg/json/encode.go
> +++ b/src/pkg/json/encode.go
> @@ -76,6 +76,25 @@
>        return buf.Bytes(), nil
>  }
>
> +// HTMLEscape re-encodes <, >, and & characters in JSON data so
> +// they may be safely embedded in HTML.  For historical reasons,
> +// browsers don't honor standard HTML escaping within <script>
> +// tags, so an alternative JSON encoding must be used.
> +func HTMLEscape(json []byte) []byte {
> +       var b bytes.Buffer
> +       for _, c := range json {
> +               switch c {
> +               case '<', '>', '&':
> +                       b.WriteString(`\u00`)
> +                       b.WriteByte(hex[c>>4])
> +                       b.WriteByte(hex[c&0xF])
> +               default:
> +                       b.WriteByte(c)
> +               }
> +       }
> +       return b.Bytes()
> +}
> +
>  // Marshaler is the interface implemented by objects that
>  // can marshal themselves into valid JSON.
>  type Marshaler interface {
>
>
>

[mstetson, 2010-06-03 19:26:01]

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

Please take another look.

[rsc1, 2010-06-30 21:49:34]

Looks good, thanks.
Sorry for the long delay.

http://codereview.appspot.com/1496042/diff/7001/8002
File src/pkg/json/encode.go (right):

http://codereview.appspot.com/1496042/diff/7001/8002#newcode91
src/pkg/json/encode.go:91: // and & characters re-encoded so  they may be safely
embedded
HTMLEscape appends to dst the JSON-encoded src with <, >, and &
characters inside string literals changed to \u003c, \u003e, \u0026,
so that the JSON will be safe to embed inside HTML <script> tags.

http://codereview.appspot.com/1496042/diff/7001/8002#newcode96
src/pkg/json/encode.go:96: start := 0
before this line (but inside the func)

    // < > & can only appear in string literals,
    // so just scan the string one byte at a time.

[mstetson, 2010-07-05 20:47:53]

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

Please take another look.

[rsc1, 2010-07-12 18:16:08]

LGTM

[rsc, 2010-07-12 18:26:49]

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

json: Add HTMLEscape

R=rsc
CC=golang-dev
http://codereview.appspot.com/1496042

Committer: Russ Cox <rsc@golang.org>