encoding/xml: predefine xml name space prefix

Also change prefix generation to use more human-friendly prefixes.

Fixes #5040.

R=golang-dev, r, bradfitz
CC=golang-dev
https://golang.org/cl/7777047

src/pkg/encoding/xml/marshal.go

@@ -126,6 +126,70 @@ type printer struct {
 	depth      int
 	indentedIn bool
 	putNewline bool
+	attrNS     map[string]string // map prefix -> name space
+	attrPrefix map[string]string // map name space -> prefix
+}
+
+// createAttrPrefix finds the name space prefix attribute to use for the given name space,
+// defining a new prefix if necessary. It returns the prefix and whether it is new.
+func (p *printer) createAttrPrefix(url string) (prefix string, isNew bool) {
+	if prefix = p.attrPrefix[url]; prefix != "" {
+		return prefix, false
+	}
+
+	// The "http://www.w3.org/XML/1998/namespace" name space is predefined as "xml"
+	// and must be referred to that way.
+	// (The "http://www.w3.org/2000/xmlns/" name space is also predefined as "xmlns",
+	// but users should not be trying to use that one directly - that's our job.)
+	if url == xmlURL {
+		return "xml", false
+	}
+
+	// Need to define a new name space.
+	if p.attrPrefix == nil {
+		p.attrPrefix = make(map[string]string)
+		p.attrNS = make(map[string]string)
+	}
+
+	// Pick a name. We try to use the final element of the path
+	// but fall back to _.
+	prefix = strings.TrimRight(url, "/")
+	if i := strings.LastIndex(prefix, "/"); i >= 0 {
+		prefix = prefix[i+1:]
+	}
+	if prefix == "" || !isName([]byte(prefix)) || strings.Contains(prefix, ":") {
+		prefix = "_"
+	}
+	if strings.HasPrefix(prefix, "xml") {
+		// xmlanything is reserved.
+		prefix = "_" + prefix
+	}
+	if p.attrNS[prefix] != "" {
+		// Name is taken. Find a better one.
+		for p.seq++; ; p.seq++ {
+			if id := prefix + "_" + strconv.Itoa(p.seq); p.attrNS[id] == "" {
+				prefix = id
+				break
+			}
+		}
+	}
+
+	p.attrPrefix[url] = prefix
+	p.attrNS[prefix] = url
+
+	p.WriteString(`xmlns:`)
+	p.WriteString(prefix)
+	p.WriteString(`="`)
+	EscapeText(p, []byte(url))
+	p.WriteString(`" `)
+
+	return prefix, true
+}
+
+// deleteAttrPrefix removes an attribute name space prefix.
+func (p *printer) deleteAttrPrefix(prefix string) {
+	delete(p.attrPrefix, p.attrNS[prefix])
+	delete(p.attrNS, prefix)
 }
 
 // marshalValue writes one or more XML elements representing val.
@@ -212,17 +276,11 @@ func (p *printer) marshalValue(val reflect.Value, finfo *fieldInfo) error {
 		}
 		p.WriteByte(' ')
 		if finfo.xmlns != "" {
-			p.WriteString("xmlns:")
-			p.seq++
-			id := "_" + strconv.Itoa(p.seq)
-			p.WriteString(id)
-			p.WriteString(`="`)
-			// TODO: EscapeString, to avoid the allocation.
-			if err := EscapeText(p, []byte(finfo.xmlns)); err != nil {
-				return err
+			prefix, created := p.createAttrPrefix(finfo.xmlns)
+			if created {
+				defer p.deleteAttrPrefix(prefix)
 			}
-			p.WriteString(`" `)
-			p.WriteString(id)
+			p.WriteString(prefix)
 			p.WriteByte(':')
 		}
 		p.WriteString(finfo.name)

src/pkg/encoding/xml/read_test.go

@@ -503,6 +503,11 @@ type TableAttrs struct {
 type TAttr struct {
 	HTable string `xml:"http://www.w3.org/TR/html4/ table,attr"`
 	FTable string `xml:"http://www.w3schools.com/furniture table,attr"`
+	Lang   string `xml:"http://www.w3.org/XML/1998/namespace lang,attr,omitempty"`
+	Other1 string `xml:"http://golang.org/xml/ other,attr,omitempty"`
+	Other2 string `xml:"http://golang.org/xmlfoo/ other,attr,omitempty"`
+	Other3 string `xml:"http://golang.org/json/ other,attr,omitempty"`
+	Other4 string `xml:"http://golang.org/2/json/ other,attr,omitempty"`
 }
 
 var tableAttrs = []struct {
@@ -514,33 +519,33 @@ var tableAttrs = []struct {
 		xml: `<TableAttrs xmlns:f="http://www.w3schools.com/furniture" xmlns:h="http://www.w3.org/TR/html4/"><TAttr ` +
 			`h:table="hello" f:table="world" ` +
 			`/></TableAttrs>`,
-		tab: TableAttrs{TAttr{"hello", "world"}},
+		tab: TableAttrs{TAttr{HTable: "hello", FTable: "world"}},
 	},
 	{
 		xml: `<TableAttrs><TAttr xmlns:f="http://www.w3schools.com/furniture" xmlns:h="http://www.w3.org/TR/html4/" ` +
 			`h:table="hello" f:table="world" ` +
 			`/></TableAttrs>`,
-		tab: TableAttrs{TAttr{"hello", "world"}},
+		tab: TableAttrs{TAttr{HTable: "hello", FTable: "world"}},
 	},
 	{
 		xml: `<TableAttrs><TAttr ` +
 			`h:table="hello" f:table="world" xmlns:f="http://www.w3schools.com/furniture" xmlns:h="http://www.w3.org/TR/html4/" ` +
 			`/></TableAttrs>`,
-		tab: TableAttrs{TAttr{"hello", "world"}},
+		tab: TableAttrs{TAttr{HTable: "hello", FTable: "world"}},
 	},
 	{
 		// Default space does not apply to attribute names.
 		xml: `<TableAttrs xmlns="http://www.w3schools.com/furniture" xmlns:h="http://www.w3.org/TR/html4/"><TAttr ` +
 			`h:table="hello" table="world" ` +
 			`/></TableAttrs>`,
-		tab: TableAttrs{TAttr{"hello", ""}},
+		tab: TableAttrs{TAttr{HTable: "hello", FTable: ""}},
 	},
 	{
 		// Default space does not apply to attribute names.
 		xml: `<TableAttrs xmlns:f="http://www.w3schools.com/furniture"><TAttr xmlns="http://www.w3.org/TR/html4/" ` +
 			`table="hello" f:table="world" ` +
 			`/></TableAttrs>`,
-		tab: TableAttrs{TAttr{"", "world"}},
+		tab: TableAttrs{TAttr{HTable: "", FTable: "world"}},
 	},
 	{
 		xml: `<TableAttrs><TAttr ` +
@@ -553,7 +558,7 @@ var tableAttrs = []struct {
 		xml: `<TableAttrs xmlns:h="http://www.w3.org/TR/html4/"><TAttr ` +
 			`h:table="hello" table="world" ` +
 			`/></TableAttrs>`,
-		tab: TableAttrs{TAttr{"hello", ""}},
+		tab: TableAttrs{TAttr{HTable: "hello", FTable: ""}},
 		ns:  "http://www.w3schools.com/furniture",
 	},
 	{
@@ -561,7 +566,7 @@ var tableAttrs = []struct {
 		xml: `<TableAttrs xmlns:f="http://www.w3schools.com/furniture"><TAttr ` +
 			`table="hello" f:table="world" ` +
 			`/></TableAttrs>`,
-		tab: TableAttrs{TAttr{"", "world"}},
+		tab: TableAttrs{TAttr{HTable: "", FTable: "world"}},
 		ns:  "http://www.w3.org/TR/html4/",
 	},
 	{
@@ -596,14 +601,23 @@ func TestUnmarshalNSAttr(t *testing.T) {
 }
 
 func TestMarshalNSAttr(t *testing.T) {
-	dst := TableAttrs{TAttr{"hello", "world"}}
-	data, err := Marshal(&dst)
+	src := TableAttrs{TAttr{"hello", "world", "en_US", "other1", "other2", "other3", "other4"}}
+	data, err := Marshal(&src)
 	if err != nil {
 		t.Fatalf("Marshal: %v", err)
 	}
-	want := `<TableAttrs><TAttr xmlns:_1="http://www.w3.org/TR/html4/" _1:table="hello" xmlns:_2="http://www.w3schools.com/furniture" _2:table="world"></TAttr></TableAttrs>`
+	want := `<TableAttrs><TAttr xmlns:html4="http://www.w3.org/TR/html4/" html4:table="hello" xmlns:furniture="http://www.w3schools.com/furniture" furniture:table="world" xml:lang="en_US" xmlns:_xml="http://golang.org/xml/" _xml:other="other1" xmlns:_xmlfoo="http://golang.org/xmlfoo/" _xmlfoo:other="other2" xmlns:json="http://golang.org/json/" json:other="other3" xmlns:json_1="http://golang.org/2/json/" json_1:other="other4"></TAttr></TableAttrs>`
 	str := string(data)
 	if str != want {
-		t.Errorf("have: %q\nwant: %q\n", str, want)
+		t.Errorf("Marshal:\nhave: %#q\nwant: %#q\n", str, want)
+	}
+
+	var dst TableAttrs
+	if err := Unmarshal(data, &dst); err != nil {
+		t.Errorf("Unmarshal: %v", err)
+	}
+
+	if dst != src {
+		t.Errorf("Unmarshal = %q, want %q", dst, src)
 	}
 }

src/pkg/encoding/xml/xml.go

@@ -273,6 +273,8 @@ func (d *Decoder) Token() (t Token, err error) {
 	return
 }
 
+const xmlURL = "http://www.w3.org/XML/1998/namespace"
+
 // Apply name space translation to name n.
 // The default name space (for Space=="")
 // applies only to element names, not to attribute names.
@@ -282,6 +284,8 @@ func (d *Decoder) translate(n *Name, isElementName bool) {
 		return
 	case n.Space == "" && !isElementName:
 		return
+	case n.Space == "xml":
+		n.Space = xmlURL
 	case n.Space == "" && n.Local == "xmlns":
 		return
 	}