Revert "encoding/xml: reject XML declaration after start of document"

This reverts commit 8a0fbd75a54c27ff2ae624ac2775bf752cdbceb4.

Reason for revert: Breaking real-world tests inside Google,
which means it probably breaks real-world tests outside Google.

One instance I have seen is a <!-- --> comment (often a copyright notice) before the procinst.

Another test checks that a canonicalizer can handle a test input that simply has procinsts mid-XML.

XML is full of contradictions, XML implementations more so. If we are going to start being picky, that probably needs to be controlled by a GODEBUG (and a proposal).

For #65691 (will reopen manually).

Change-Id: Ib52d0944b1478e71744a2a35b271fdf7e1c972ca
Reviewed-on: https://go-review.googlesource.com/c/go/+/570175
Reviewed-by: Than McIntosh <thanm@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Russ Cox <rsc@golang.org>

diff --git a/src/encoding/xml/marshal.go b/src/encoding/xml/marshal.go
index c30f0dca1514dfb809d28213b8445478aa2fb933..05b5542dfb41626b77974b902aadc584d17f7b4d 100644 (file)
--- a/src/encoding/xml/marshal.go
+++ b/src/encoding/xml/marshal.go
@@ -208,6 +208,7 @@ var (
 // EncodeToken allows writing a [ProcInst] with Target set to "xml" only as the first token
 // in the stream.
 func (enc *Encoder) EncodeToken(t Token) error {
+
        p := &enc.p
        switch t := t.(type) {
        case StartElement:
@@ -227,10 +228,11 @@ func (enc *Encoder) EncodeToken(t Token) error {
                p.WriteString("<!--")
                p.Write(t)
                p.WriteString("-->")
+               return p.cachedWriteError()
        case ProcInst:
                // First token to be encoded which is also a ProcInst with target of xml
                // is the xml declaration. The only ProcInst where target of xml is allowed.
-               if t.Target == "xml" && p.wroteNonWS {
+               if t.Target == "xml" && p.w.Buffered() != 0 {
                        return fmt.Errorf("xml: EncodeToken of ProcInst xml target only valid for xml declaration, first token encoded")
                }
                if !isNameString(t.Target) {
@@ -255,30 +257,9 @@ func (enc *Encoder) EncodeToken(t Token) error {
                p.WriteString(">")
        default:
                return fmt.Errorf("xml: EncodeToken of invalid token type")
-       }
-       if err := p.cachedWriteError(); err != nil || enc.p.wroteNonWS {
-               return err
-       }
-       enc.p.wroteNonWS = !isWhitespace(t)
-       return nil
-}
 
-// isWhitespace reports whether t is a CharData token consisting entirely of
-// XML whitespace.
-func isWhitespace(t Token) bool {
-       switch t := t.(type) {
-       case CharData:
-               for _, b := range t {
-                       switch b {
-                       case ' ', '\r', '\n', '\t':
-                       default:
-                               return false
-                       }
-               }
-               return true
-       default:
-               return false
        }
+       return p.cachedWriteError()
 }
 
 // isValidDirective reports whether dir is a valid directive text,
@@ -348,7 +329,6 @@ type printer struct {
        prefixes   []string
        tags       []Name
        closed     bool
-       wroteNonWS bool
        err        error
 }
 

diff --git a/src/encoding/xml/marshal_test.go b/src/encoding/xml/marshal_test.go
index 88918d4552b248e8c596e092b52fc19028297833..b8bce7170a60b6f350c7f0dc5fd50f14d570237c 100644 (file)
--- a/src/encoding/xml/marshal_test.go
+++ b/src/encoding/xml/marshal_test.go
@@ -2356,9 +2356,6 @@ func TestProcInstEncodeToken(t *testing.T) {
        var buf bytes.Buffer
        enc := NewEncoder(&buf)
 
-       if err := enc.EncodeToken(CharData(" \n\r\t")); err != nil {
-               t.Fatal("enc.EncodeToken: expected to be able to encode whitespace as first token")
-       }
        if err := enc.EncodeToken(ProcInst{"xml", []byte("Instruction")}); err != nil {
                t.Fatalf("enc.EncodeToken: expected to be able to encode xml target ProcInst as first token, %s", err)
        }

diff --git a/src/encoding/xml/xml.go b/src/encoding/xml/xml.go
index 582cfee222be5ba6753fef06ff47f1bf20d40a56..0fe323f7c86afc36e3ac0297e6d9ac2db94575b1 100644 (file)
--- a/src/encoding/xml/xml.go
+++ b/src/encoding/xml/xml.go
@@ -212,7 +212,6 @@ type Decoder struct {
        line           int
        linestart      int64
        offset         int64
-       readNonWS      bool
        unmarshalDepth int
 }
 
@@ -564,8 +563,6 @@ func (d *Decoder) rawToken() (Token, error) {
                return EndElement{d.toClose}, nil
        }
 
-       readNonWS := d.readNonWS
-
        b, ok := d.getc()
        if !ok {
                return nil, d.err
@@ -578,12 +575,8 @@ func (d *Decoder) rawToken() (Token, error) {
                if data == nil {
                        return nil, d.err
                }
-               if !d.readNonWS && !isWhitespace(CharData(data)) {
-                       d.readNonWS = true
-               }
                return CharData(data), nil
        }
-       d.readNonWS = true
 
        if b, ok = d.mustgetc(); !ok {
                return nil, d.err
@@ -634,11 +627,6 @@ func (d *Decoder) rawToken() (Token, error) {
                data = data[0 : len(data)-2] // chop ?>
 
                if target == "xml" {
-                       if readNonWS {
-                               d.err = errors.New("xml: XML declaration after start of document")
-                               return nil, d.err
-                       }
-
                        content := string(data)
                        ver := procInst("version", content)
                        if ver != "" && ver != "1.0" {

diff --git a/src/encoding/xml/xml_test.go b/src/encoding/xml/xml_test.go
index c3848c387301b6e880a806dc97686a2babde19d2..a6763fd547ddfcbef3a3b07cf1321667c528c00c 100644 (file)
--- a/src/encoding/xml/xml_test.go
+++ b/src/encoding/xml/xml_test.go
@@ -1352,12 +1352,10 @@ func TestParseErrors(t *testing.T) {
 
                // Header-related errors.
                {`<?xml version="1.1" encoding="UTF-8"?>`, `unsupported version "1.1"; only version 1.0 is supported`},
-               {`<foo><?xml version="1.0"?>`, `XML declaration after start of document`},
 
                // Cases below are for "no errors".
                {withDefaultHeader(`<?ok?>`), ``},
                {withDefaultHeader(`<?ok version="ok"?>`), ``},
-               {`  <?xml version="1.0"?>`, ``},
        }
 
        for _, test := range tests {