encoding/xml: improve the test coverage, fix minor bugs

Improve the test coverage of encoding/xml package by adding
the test cases for the execution paths that were not covered before.

Since it reveals a couple of issues, fix them as well while we're at it.

As I used an `strings.EqualFold` instead of adding one more `strings.ToLower`,
our fix to `autoClose()` tends to run faster as well as a result.

	name             old time/op    new time/op    delta
	HTMLAutoClose-8    5.93µs ± 2%    5.75µs ± 3%  -3.16%  (p=0.000 n=10+10)
	name             old alloc/op   new alloc/op   delta
	HTMLAutoClose-8    2.60kB ± 0%    2.58kB ± 0%  -0.46%  (p=0.000 n=10+10)
	name             old allocs/op  new allocs/op  delta
	HTMLAutoClose-8      72.0 ± 0%      67.0 ± 0%  -6.94%  (p=0.000 n=10+10)

The overall `encoding/xml` test coverage increase is `88.1% -> 89.9%`;
although it may look insignificant, this CL covers some important corner cases,
like `autoClose()` functionality (that was not tested at all).

Fixes #49635
Fixes #49636

Change-Id: I50b2769896c197eb285672313b7148f4fe8bdb38
Reviewed-on: https://go-review.googlesource.com/c/go/+/364734
Trust: Bryan Mills <bcmills@google.com>
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
Trust: Daniel Martí <mvdan@mvdan.cc>
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gopher Robot <gobot@golang.org>

diff --git a/src/encoding/xml/xml.go b/src/encoding/xml/xml.go
index 8a0a9c253ade3994aa4aa35f253057f7fc24643e..ef51252dcbbb1796fe1a32607573591484bdb70e 100644 (file)
--- a/src/encoding/xml/xml.go
+++ b/src/encoding/xml/xml.go
@@ -10,9 +10,6 @@ package xml
 //    Annotated XML spec: https://www.xml.com/axml/testaxml.htm
 //    XML name spaces: https://www.w3.org/TR/REC-xml-names/
 
-// TODO(rsc):
-//     Test error handling.
-
 import (
        "bufio"
        "bytes"
@@ -499,7 +496,7 @@ func (d *Decoder) popElement(t *EndElement) bool {
                return false
        case s.name.Space != name.Space:
                d.err = d.syntaxError("element <" + s.name.Local + "> in space " + s.name.Space +
-                       "closed by </" + name.Local + "> in space " + name.Space)
+                       " closed by </" + name.Local + "> in space " + name.Space)
                return false
        }
 
@@ -523,12 +520,11 @@ func (d *Decoder) autoClose(t Token) (Token, bool) {
        if d.stk == nil || d.stk.kind != stkStart {
                return nil, false
        }
-       name := strings.ToLower(d.stk.name.Local)
        for _, s := range d.AutoClose {
-               if strings.ToLower(s) == name {
+               if strings.EqualFold(s, d.stk.name.Local) {
                        // This one should be auto closed if t doesn't close it.
                        et, ok := t.(EndElement)
-                       if !ok || et.Name.Local != name {
+                       if !ok || !strings.EqualFold(et.Name.Local, d.stk.name.Local) {
                                return EndElement{d.stk.name}, true
                        }
                        break

diff --git a/src/encoding/xml/xml_test.go b/src/encoding/xml/xml_test.go
index 19152dbdb689511a4f3ec28eab5ee21172438472..ab1dbf849bfd69e70ccf3b372833cba53305d96e 100644 (file)
--- a/src/encoding/xml/xml_test.go
+++ b/src/encoding/xml/xml_test.go
@@ -673,6 +673,19 @@ func TestCopyTokenStartElement(t *testing.T) {
        }
 }
 
+func TestCopyTokenComment(t *testing.T) {
+       data := []byte("<!-- some comment -->")
+       var tok1 Token = Comment(data)
+       tok2 := CopyToken(tok1)
+       if !reflect.DeepEqual(tok1, tok2) {
+               t.Error("CopyToken(Comment) != Comment")
+       }
+       data[1] = 'o'
+       if reflect.DeepEqual(tok1, tok2) {
+               t.Error("CopyToken(Comment) uses same buffer.")
+       }
+}
+
 func TestSyntaxErrorLineNum(t *testing.T) {
        testInput := "<P>Foo<P>\n\n<P>Bar</>\n"
        d := NewDecoder(strings.NewReader(testInput))
@@ -1060,3 +1073,155 @@ func TestRoundTrip(t *testing.T) {
                t.Run(name, func(t *testing.T) { testRoundTrip(t, input) })
        }
 }
+
+func TestParseErrors(t *testing.T) {
+       withDefaultHeader := func(s string) string {
+               return `<?xml version="1.0" encoding="UTF-8"?>` + s
+       }
+       tests := []struct {
+               src string
+               err string
+       }{
+               {withDefaultHeader(`</foo>`), `unexpected end element </foo>`},
+               {withDefaultHeader(`<x:foo></y:foo>`), `element <foo> in space x closed by </foo> in space y`},
+               {withDefaultHeader(`<? not ok ?>`), `expected target name after <?`},
+               {withDefaultHeader(`<!- not ok -->`), `invalid sequence <!- not part of <!--`},
+               {withDefaultHeader(`<!-? not ok -->`), `invalid sequence <!- not part of <!--`},
+               {withDefaultHeader(`<![not ok]>`), `invalid <![ sequence`},
+               {withDefaultHeader("\xf1"), `invalid UTF-8`},
+
+               // Header-related errors.
+               {`<?xml version="1.1" encoding="UTF-8"?>`, `unsupported version "1.1"; only version 1.0 is supported`},
+
+               // Cases below are for "no errors".
+               {withDefaultHeader(`<?ok?>`), ``},
+               {withDefaultHeader(`<?ok version="ok"?>`), ``},
+       }
+
+       for _, test := range tests {
+               d := NewDecoder(strings.NewReader(test.src))
+               var err error
+               for {
+                       _, err = d.Token()
+                       if err != nil {
+                               break
+                       }
+               }
+               if test.err == "" {
+                       if err != io.EOF {
+                               t.Errorf("parse %s: have %q error, expected none", test.src, err)
+                       }
+                       continue
+               }
+               if err == nil || err == io.EOF {
+                       t.Errorf("parse %s: have no error, expected a non-nil error", test.src)
+                       continue
+               }
+               if !strings.Contains(err.Error(), test.err) {
+                       t.Errorf("parse %s: can't find %q error sudbstring\nerror: %q", test.src, test.err, err)
+                       continue
+               }
+       }
+}
+
+const testInputHTMLAutoClose = `<?xml version="1.0" encoding="UTF-8"?>
+<br>
+<br/><br/>
+<br><br>
+<br></br>
+<BR>
+<BR/><BR/>
+<Br></Br>
+<BR><span id="test">abc</span><br/><br/>`
+
+func BenchmarkHTMLAutoClose(b *testing.B) {
+       b.RunParallel(func(p *testing.PB) {
+               for p.Next() {
+                       d := NewDecoder(strings.NewReader(testInputHTMLAutoClose))
+                       d.Strict = false
+                       d.AutoClose = HTMLAutoClose
+                       d.Entity = HTMLEntity
+                       for {
+                               _, err := d.Token()
+                               if err != nil {
+                                       if err == io.EOF {
+                                               break
+                                       }
+                                       b.Fatalf("unexpected error: %v", err)
+                               }
+                       }
+               }
+       })
+}
+
+func TestHTMLAutoClose(t *testing.T) {
+       wantTokens := []Token{
+               ProcInst{"xml", []byte(`version="1.0" encoding="UTF-8"`)},
+               CharData("\n"),
+               StartElement{Name{"", "br"}, []Attr{}},
+               EndElement{Name{"", "br"}},
+               CharData("\n"),
+               StartElement{Name{"", "br"}, []Attr{}},
+               EndElement{Name{"", "br"}},
+               StartElement{Name{"", "br"}, []Attr{}},
+               EndElement{Name{"", "br"}},
+               CharData("\n"),
+               StartElement{Name{"", "br"}, []Attr{}},
+               EndElement{Name{"", "br"}},
+               StartElement{Name{"", "br"}, []Attr{}},
+               EndElement{Name{"", "br"}},
+               CharData("\n"),
+               StartElement{Name{"", "br"}, []Attr{}},
+               EndElement{Name{"", "br"}},
+               CharData("\n"),
+               StartElement{Name{"", "BR"}, []Attr{}},
+               EndElement{Name{"", "BR"}},
+               CharData("\n"),
+               StartElement{Name{"", "BR"}, []Attr{}},
+               EndElement{Name{"", "BR"}},
+               StartElement{Name{"", "BR"}, []Attr{}},
+               EndElement{Name{"", "BR"}},
+               CharData("\n"),
+               StartElement{Name{"", "Br"}, []Attr{}},
+               EndElement{Name{"", "Br"}},
+               CharData("\n"),
+               StartElement{Name{"", "BR"}, []Attr{}},
+               EndElement{Name{"", "BR"}},
+               StartElement{Name{"", "span"}, []Attr{{Name: Name{"", "id"}, Value: "test"}}},
+               CharData("abc"),
+               EndElement{Name{"", "span"}},
+               StartElement{Name{"", "br"}, []Attr{}},
+               EndElement{Name{"", "br"}},
+               StartElement{Name{"", "br"}, []Attr{}},
+               EndElement{Name{"", "br"}},
+       }
+
+       d := NewDecoder(strings.NewReader(testInputHTMLAutoClose))
+       d.Strict = false
+       d.AutoClose = HTMLAutoClose
+       d.Entity = HTMLEntity
+       var haveTokens []Token
+       for {
+               tok, err := d.Token()
+               if err != nil {
+                       if err == io.EOF {
+                               break
+                       }
+                       t.Fatalf("unexpected error: %v", err)
+               }
+               haveTokens = append(haveTokens, CopyToken(tok))
+       }
+       if len(haveTokens) != len(wantTokens) {
+               t.Errorf("tokens count mismatch: have %d, want %d", len(haveTokens), len(wantTokens))
+       }
+       for i, want := range wantTokens {
+               if i >= len(haveTokens) {
+                       t.Errorf("token[%d] expected %#v, have no token", i, want)
+               } else {
+                       have := haveTokens[i]
+                       if !reflect.DeepEqual(have, want) {
+                               t.Errorf("token[%d] mismatch:\nhave: %#v\nwant: %#v", i, have, want)
+                       }
+               }
+       }
+}