encoding/json: fix handling of null with ,string fields

Fixes #8587.

LGTM=bradfitz
R=golang-codereviews, bradfitz
CC=golang-codereviews, iant, r
https://golang.org/cl/152270044

diff --git a/src/encoding/json/decode.go b/src/encoding/json/decode.go
index 67ec37388f8bb636ce703233b24e7c0203bc31c5..705bc2e17a7e3455c588fca9a4d2556782830010 100644 (file)
--- a/src/encoding/json/decode.go
+++ b/src/encoding/json/decode.go
@@ -173,7 +173,6 @@ type decodeState struct {
        scan       scanner
        nextscan   scanner // for calls to nextValue
        savedError error
-       tempstr    string // scratch space to avoid some allocations
        useNumber  bool
 }
 
@@ -293,6 +292,32 @@ func (d *decodeState) value(v reflect.Value) {
        }
 }
 
+type unquotedValue struct{}
+
+// valueQuoted is like value but decodes a
+// quoted string literal or literal null into an interface value.
+// If it finds anything other than a quoted string literal or null,
+// valueQuoted returns unquotedValue{}.
+func (d *decodeState) valueQuoted() interface{} {
+       switch op := d.scanWhile(scanSkipSpace); op {
+       default:
+               d.error(errPhase)
+
+       case scanBeginArray:
+               d.array(reflect.Value{})
+
+       case scanBeginObject:
+               d.object(reflect.Value{})
+
+       case scanBeginLiteral:
+               switch v := d.literalInterface().(type) {
+               case nil, string:
+                       return v
+               }
+       }
+       return unquotedValue{}
+}
+
 // indirect walks down v allocating pointers as needed,
 // until it gets to a non-pointer.
 // if it encounters an Unmarshaler, indirect stops and returns that.
@@ -444,6 +469,8 @@ func (d *decodeState) array(v reflect.Value) {
        }
 }
 
+var nullLiteral = []byte("null")
+
 // object consumes an object from d.data[d.off-1:], decoding into the value v.
 // the first byte ('{') of the object has been read already.
 func (d *decodeState) object(v reflect.Value) {
@@ -566,9 +593,14 @@ func (d *decodeState) object(v reflect.Value) {
 
                // Read value.
                if destring {
-                       d.value(reflect.ValueOf(&d.tempstr))
-                       d.literalStore([]byte(d.tempstr), subv, true)
-                       d.tempstr = "" // Zero scratch space for successive values.
+                       switch qv := d.valueQuoted().(type) {
+                       case nil:
+                               d.literalStore(nullLiteral, subv, false)
+                       case string:
+                               d.literalStore([]byte(qv), subv, true)
+                       default:
+                               d.saveError(fmt.Errorf("json: invalid use of ,string struct tag, trying to unmarshal unquoted value into %v", item, v.Type()))
+                       }
                } else {
                        d.value(subv)
                }

diff --git a/src/encoding/json/decode_test.go b/src/encoding/json/decode_test.go
index d95657d7292984b259e27b817a478a76f952eec8..7235969b9fef556964823bfd2ac7550d256daada 100644 (file)
--- a/src/encoding/json/decode_test.go
+++ b/src/encoding/json/decode_test.go
@@ -1070,18 +1070,25 @@ func TestEmptyString(t *testing.T) {
        }
 }
 
-// Test that the returned error is non-nil when trying to unmarshal null string into int, for successive ,string option
-// Issue 7046
+// Test that a null for ,string is not replaced with the previous quoted string (issue 7046).
+// It should also not be an error (issue 2540, issue 8587).
 func TestNullString(t *testing.T) {
        type T struct {
-               A int `json:",string"`
-               B int `json:",string"`
+               A int  `json:",string"`
+               B int  `json:",string"`
+               C *int `json:",string"`
        }
-       data := []byte(`{"A": "1", "B": null}`)
+       data := []byte(`{"A": "1", "B": null, "C": null}`)
        var s T
+       s.B = 1
+       s.C = new(int)
+       *s.C = 2
        err := Unmarshal(data, &s)
-       if err == nil {
-               t.Fatalf("expected error; got %v", s)
+       if err != nil {
+               t.Fatalf("Unmarshal: %v")
+       }
+       if s.B != 1 || s.C != nil {
+               t.Fatalf("after Unmarshal, s.B=%d, s.C=%p, want 1, nil", s.B, s.C)
        }
 }