[release-branch.go1.8] encoding/xml: disable checking of attribute syntax, like Go 1.7

Consider this struct, which expects an attribute A and a child C both ints:

    type X struct {
        XMLName xml.Name `xml:"X"`
        A       int      `xml:",attr"`
        C       int
    }

Go 1.2 through Go 1.7 were consistent: attributes unchecked,
children strictly checked:

    $ go1.7 run /tmp/x.go
    <X></X>              ok
    <X A=""></X>         ok
    <X A="bad"></X>      ok
    <X></X>              ok
    <X><C></C></X>       ERROR strconv.ParseInt: parsing "": invalid syntax
    <X><C/></X>          ERROR strconv.ParseInt: parsing "": invalid syntax
    <X><C>bad</C></X>    ERROR strconv.ParseInt: parsing "bad": invalid syntax
    $

Go 1.8 made attributes strictly checked, matching children:

    $ go1.8 run /tmp/x.go
    <X></X>              ok
    <X A=""></X>         ERROR strconv.ParseInt: parsing "": invalid syntax
    <X A="bad"></X>      ERROR strconv.ParseInt: parsing "bad": invalid syntax
    <X></X>              ok
    <X><C></C></X>       ERROR strconv.ParseInt: parsing "": invalid syntax
    <X><C/></X>          ERROR strconv.ParseInt: parsing "": invalid syntax
    <X><C>bad</C></X>    ERROR strconv.ParseInt: parsing "bad": invalid syntax
    $

but this broke XML code that had empty attributes (#19333).

In Go 1.9 we plan to start allowing empty children (#13417).
The fix for that will also make empty attributes work again:

    $ go run /tmp/x.go  # Go 1.9 development
    <X></X>              ok
    <X A=""></X>         ok
    <X A="bad"></X>      ERROR strconv.ParseInt: parsing "bad": invalid syntax
    <X></X>              ok
    <X><C></C></X>       ok
    <X><C/></X>          ok
    <X><C>bad</C></X>    ERROR strconv.ParseInt: parsing "bad": invalid syntax
    $

For Go 1.8.1, we want to restore the empty attribute behavior
to match Go 1.7 but not yet change the child behavior as planned for Go 1.9,
since that change hasn't been through release testing.

Instead, restore the more lax Go 1.7 behavior, so that XML files
with empty attributes will not be broken until Go 1.9:

    $ go run /tmp/x.go  # after this CL
    <X></X>              ok
    <X A=""></X>         ok
    <X A="bad"></X>      ok
    <X></X>              ok
    <X><C></C></X>       ERROR strconv.ParseInt: parsing "": invalid syntax
    <X><C/></X>          ERROR strconv.ParseInt: parsing "": invalid syntax
    <X><C>bad</C></X>    ERROR strconv.ParseInt: parsing "bad": invalid syntax
    $

Fixes #19333.

Change-Id: I3d38ebd2509f5b6ea3fd4856327f887f9a1a8085
Reviewed-on: https://go-review.googlesource.com/39607
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Sarah Adams <shadams@google.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>

diff --git a/src/encoding/xml/marshal_test.go b/src/encoding/xml/marshal_test.go
index 5ec7ececa4d039331bfec347269bccf2e2b71e31..0126146d332e0e098450074325228a84401e1e40 100644 (file)
--- a/src/encoding/xml/marshal_test.go
+++ b/src/encoding/xml/marshal_test.go
@@ -2428,7 +2428,10 @@ func TestIssue16158(t *testing.T) {
        err := Unmarshal([]byte(data), &struct {
                B byte `xml:"b,attr,omitempty"`
        }{})
-       if err == nil {
-               t.Errorf("Unmarshal: expected error, got nil")
+
+       // For Go 1.8.1 we've restored the old "no errors reported" behavior.
+       // We'll try again in Go 1.9 to report errors.
+       if err != nil {
+               t.Errorf("Unmarshal: expected nil, got error")
        }
 }

diff --git a/src/encoding/xml/read.go b/src/encoding/xml/read.go
index 5a89d5f504aeba6cea7579583729a4684be6a44d..799b57e9d1515a09c81d8f8932f21871bf7c567c 100644 (file)
--- a/src/encoding/xml/read.go
+++ b/src/encoding/xml/read.go
@@ -285,7 +285,8 @@ func (p *Decoder) unmarshalAttr(val reflect.Value, attr Attr) error {
                return nil
        }
 
-       return copyValue(val, []byte(attr.Value))
+       copyValue(val, []byte(attr.Value))
+       return nil
 }
 
 var (

diff --git a/src/encoding/xml/xml_test.go b/src/encoding/xml/xml_test.go
index dad6ed98c1bffe1f45b82001b14184937c524112..f43a5e7eebc83cd9f6592b7ac5a9f7017bcfb9d4 100644 (file)
--- a/src/encoding/xml/xml_test.go
+++ b/src/encoding/xml/xml_test.go
@@ -797,3 +797,37 @@ func TestIssue12417(t *testing.T) {
                }
        }
 }
+
+func TestIssue19333(t *testing.T) {
+       type X struct {
+               XMLName Name `xml:"X"`
+               A       int  `xml:",attr"`
+               C       int
+       }
+
+       var tests = []struct {
+               input string
+               ok    bool
+       }{
+               {`<X></X>`, true},
+               {`<X A=""></X>`, true},
+               {`<X A="bad"></X>`, true},
+               {`<X></X>`, true},
+               {`<X><C></C></X>`, false},
+               {`<X><C/></X>`, false},
+               {`<X><C>bad</C></X>`, false},
+       }
+
+       for _, tt := range tests {
+               err := Unmarshal([]byte(tt.input), new(X))
+               if tt.ok {
+                       if err != nil {
+                               t.Errorf("%s: unexpected error: %v", tt.input, err)
+                       }
+               } else {
+                       if err == nil {
+                               t.Errorf("%s: unexpected success", tt.input)
+                       }
+               }
+       }
+}