[golang-dev] code review 6997045: encoding/json: A JSON tag can be any valid JSON string. (issue 6997045)

Reviewers: golang-dev_googlegroups.com,

Message:
Hello golang-dev@googlegroups.com,

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


Description:
encoding/json: A JSON tag can be any valid JSON string.

Fixes issue 3887.

Please review this at https://codereview.appspot.com/6997045/

Affected files:
M src/pkg/encoding/json/encode.go
M src/pkg/encoding/json/encode_test.go
M src/pkg/encoding/json/tagkey_test.go


Index: src/pkg/encoding/json/encode.go
===================================================================
--- a/src/pkg/encoding/json/encode.go
+++ b/src/pkg/encoding/json/encode.go
@@ -17,9 +17,7 @@
"runtime"
"sort"
"strconv"
- "strings"
"sync"
- "unicode"
"unicode/utf8"
)

@@ -431,23 +429,39 @@
return
}

+func isValidString(s string) bool {
+ escape := false // have we read a '\' character?
+
+ for i := 0; i < len(s); {
+ if b := s[i]; b < utf8.RuneSelf {
+ switch {
+ case b == '\\' && escape == false:
+ escape = true
+ i++
+ continue
+ case b == '"' && escape == false:
+ case b < 0x20:
+ return false
+ }
+
+ escape = false
+ i++
+ continue
+ }
+ c, size := utf8.DecodeRuneInString(s[i:])
+ if c == utf8.RuneError && size == 1 {
+ return false
+ }
+ i += size
+ }
+ return escape == false
+}
+
func isValidTag(s string) bool {
if s == "" {
return false
}
- for _, c := range s {
- switch {
- case strings.ContainsRune("!#$%&()*+-./:<=>?@[]^_{|}~", c):
- // Backslash and quote chars are reserved, but
- // otherwise any punctuation chars are allowed
- // in a tag name.
- default:
- if !unicode.IsLetter(c) && !unicode.IsDigit(c) {
- return false
- }
- }
- }
- return true
+ return isValidString(s)
}

func fieldByIndex(v reflect.Value, index []int) reflect.Value {
Index: src/pkg/encoding/json/encode_test.go
===================================================================
--- a/src/pkg/encoding/json/encode_test.go
+++ b/src/pkg/encoding/json/encode_test.go
@@ -84,6 +84,28 @@
}
}

+type ValidTag struct {
+ Space bool `json:"with space"`
+ Unicode bool `json:"Ελλάδα"`
+}
+
+var validTagExpected = `{
+ "with space": false,
+ "Ελλάδα": false
+}`
+
+func TestValidTag(t *testing.T) {
+ var s ValidTag
+ got, err := MarshalIndent(&s, "", " ")
+ if err != nil {
+ t.Fatal(err)
+ }
+
+ if got := string(got); got != validTagExpected {
+ t.Fatalf(" got: %s\nwant: %s\n", got, validTagExpected)
+ }
+}
+
// byte slices are special even if they're renamed types.
type renamedByte byte
type renamedByteSlice []byte
Index: src/pkg/encoding/json/tagkey_test.go
===================================================================
--- a/src/pkg/encoding/json/tagkey_test.go
+++ b/src/pkg/encoding/json/tagkey_test.go
@@ -56,8 +56,12 @@
Y string `:"BadFormat"`
}

-type badCodeTag struct {
- Z string `json:" !\"#&'()*+,."`
+type spaceTag struct {
+ Z string `json:"With space"`
+}
+
+type unicodeTag struct {
+ Q string `json:"Ελλάδα"`
}

var structTagObjectKeyTests = []struct {
@@ -75,9 +79,10 @@
{emptyTag{"Pour Moi"}, "Pour Moi", "W"},
{misnamedTag{"Animal Kingdom"}, "Animal Kingdom", "X"},
{badFormatTag{"Orfevre"}, "Orfevre", "Y"},
- {badCodeTag{"Reliable Man"}, "Reliable Man", "Z"},
{percentSlashTag{"brut"}, "brut", "text/html%"},
{punctuationTag{"Union Rags"}, "Union Rags", "!#$%&()*+-./:<=>?@[]^_{|
}~"},
+ {spaceTag{"Perreddu"}, "Perreddu", "With space"},
+ {unicodeTag{"Loukanikos"}, "Loukanikos", "Ελλάδα"},
}

func TestStructTagObjectKey(t *testing.T) {