code review 162340043: encoding/json: encode \t as \t instead of \u0009

encoding/json: encode \t as \t instead of \u0009

Shorter and easier to read form for a common character.

[rsc, 2014-10-27 19:55:49]

Hello adg (cc: golang-codereviews@googlegroups.com, zimmski@gmail.com),

I'd like you to review this change to
https://code.google.com/p/go/

[bradfitz, 2014-10-27 19:57:06]

LGTM
 On Oct 27, 2014 12:55 PM, <rsc@golang.org> wrote:

> Reviewers: adg,
>
> Message:
> Hello adg (cc: golang-codereviews@googlegroups.com, zimmski@gmail.com),
>
> I'd like you to review this change to
> https://code.google.com/p/go/
>
>
> Description:
> encoding/json: encode \t as \t instead of \u0009
>
> Shorter and easier to read form for a common character.
>
> Please review this at https://codereview.appspot.com/162340043/
>
> Affected files (+59, -1 lines):
>   M src/encoding/json/encode.go
>   M src/encoding/json/encode_test.go
>
>
> Index: src/encoding/json/encode.go
> ===================================================================
> --- a/src/encoding/json/encode.go
> +++ b/src/encoding/json/encode.go
> @@ -805,6 +805,9 @@
>                         case '\r':
>                                 e.WriteByte('\\')
>                                 e.WriteByte('r')
> +                       case '\t':
> +                               e.WriteByte('\\')
> +                               e.WriteByte('t')
>                         default:
>                                 // This encodes bytes < 0x20 except for \n
> and \r,
>                                 // as well as <, > and &. The latter are
> escaped because they
> @@ -878,9 +881,12 @@
>                         case '\r':
>                                 e.WriteByte('\\')
>                                 e.WriteByte('r')
> +                       case '\t':
> +                               e.WriteByte('\\')
> +                               e.WriteByte('t')
>                         default:
>                                 // This encodes bytes < 0x20 except for \n
> and \r,
> -                               // as well as < and >. The latter are
> escaped because they
> +                               // as well as <, >, and &. The latter are
> escaped because they
>                                 // can lead to security holes when
> user-controlled strings
>                                 // are rendered into JSON and served to
> some browsers.
>                                 e.WriteString(`\u00`)
> Index: src/encoding/json/encode_test.go
> ===================================================================
> --- a/src/encoding/json/encode_test.go
> +++ b/src/encoding/json/encode_test.go
> @@ -478,3 +478,55 @@
>                 t.Fatalf("*N = %d; want 42", *back.N)
>         }
>  }
> +
> +var encodeStringTests = []struct {
> +       in  string
> +       out string
> +}{
> +       {"\x00", `"\u0000"`},
> +       {"\x01", `"\u0001"`},
> +       {"\x02", `"\u0002"`},
> +       {"\x03", `"\u0003"`},
> +       {"\x04", `"\u0004"`},
> +       {"\x05", `"\u0005"`},
> +       {"\x06", `"\u0006"`},
> +       {"\x07", `"\u0007"`},
> +       {"\x08", `"\u0008"`},
> +       {"\x09", `"\t"`},
> +       {"\x0a", `"\n"`},
> +       {"\x0b", `"\u000b"`},
> +       {"\x0c", `"\u000c"`},
> +       {"\x0d", `"\r"`},
> +       {"\x0e", `"\u000e"`},
> +       {"\x0f", `"\u000f"`},
> +       {"\x10", `"\u0010"`},
> +       {"\x11", `"\u0011"`},
> +       {"\x12", `"\u0012"`},
> +       {"\x13", `"\u0013"`},
> +       {"\x14", `"\u0014"`},
> +       {"\x15", `"\u0015"`},
> +       {"\x16", `"\u0016"`},
> +       {"\x17", `"\u0017"`},
> +       {"\x18", `"\u0018"`},
> +       {"\x19", `"\u0019"`},
> +       {"\x1a", `"\u001a"`},
> +       {"\x1b", `"\u001b"`},
> +       {"\x1c", `"\u001c"`},
> +       {"\x1d", `"\u001d"`},
> +       {"\x1e", `"\u001e"`},
> +       {"\x1f", `"\u001f"`},
> +}
> +
> +func TestEncodeString(t *testing.T) {
> +       for _, tt := range encodeStringTests {
> +               b, err := Marshal(tt.in)
> +               if err != nil {
> +                       t.Errorf("Marshal(%q): %v", tt.in, err)
> +                       continue
> +               }
> +               out := string(b)
> +               if out != tt.out {
> +                       t.Errorf("Marshal(%q) = %#q, want %#q", tt.in,
> out, tt.out)
> +               }
> +       }
> +}
>
>
> --
> You received this message because you are subscribed to the Google Groups
> "golang-codereviews" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to golang-codereviews+unsubscribe@googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.
>

[rsc, 2014-10-27 22:58:31]

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

encoding/json: encode \t as \t instead of \u0009

Shorter and easier to read form for a common character.

LGTM=bradfitz
R=adg, bradfitz
CC=golang-codereviews, zimmski
https://codereview.appspot.com/162340043

[gobot, 2014-10-27 23:00:49]

This CL appears to have broken the linux-amd64 builder.
See http://build.golang.org/log/cb88d5667932be6293bad8c255131dc8fcc4a5ae

[adg, 2014-10-27 23:07:10]

This failure is real.

On Tue Oct 28 2014 at 10:00:53 AM <gobot@golang.org> wrote:

> This CL appears to have broken the linux-amd64 builder.
> See http://build.golang.org/log/cb88d5667932be6293bad8c255131dc8fcc4a5ae
>
> https://codereview.appspot.com/162340043/
>