src/encoding/xml/read.go | 28 +++++++++++++---------------
src/encoding/xml/read_test.go | 154 +++++++++++++++++++++++++++++++++++++++++++++++++++++
src/encoding/xml/xml.go | 11 +++++++++--
diff --git a/src/encoding/xml/read.go b/src/encoding/xml/read.go
index d3cb74b2c4311ae11dd57ef99c1c181023d812af..c9afc03a09d7bca208424f016338509652c4ace3 100644
--- a/src/encoding/xml/read.go
+++ b/src/encoding/xml/read.go
@@ -153,7 +153,7 @@
if val.IsNil() {
return errors.New("nil pointer passed to Unmarshal")
}
- return d.unmarshal(val.Elem(), start, 0)
+ return d.unmarshal(val.Elem(), start)
}
// An UnmarshalError represents an error in the unmarshaling process.
@@ -207,9 +207,11 @@ func (d *Decoder) unmarshalInterface(val Unmarshaler, start *StartElement) error {
// Record that decoder must stop at end tag corresponding to start.
d.pushEOF()
- d.unmarshalDepth++
+ savedInUnmarshalXML := d.inUnmarshalXML
+ d.inUnmarshalXML = true
+ defer func() { d.inUnmarshalXML = savedInUnmarshalXML }()
+
err := val.UnmarshalXML(d, *start)
- d.unmarshalDepth--
if err != nil {
d.popEOF()
return err
@@ -321,8 +323,8 @@
var errUnmarshalDepth = errors.New("exceeded max depth")
// Unmarshal a single XML element into val.
-func (d *Decoder) unmarshal(val reflect.Value, start *StartElement, depth int) error {
- if depth >= maxUnmarshalDepth || runtime.GOARCH == "wasm" && depth >= maxUnmarshalDepthWasm {
+func (d *Decoder) unmarshal(val reflect.Value, start *StartElement) error {
+ if d.stkDepth > maxUnmarshalDepth || runtime.GOARCH == "wasm" && d.stkDepth > maxUnmarshalDepthWasm {
return errUnmarshalDepth
}
// Find start element if we need it.
@@ -426,7 +428,7 @@ v.Grow(1)
v.SetLen(n + 1)
// Recur to read element into slice.
- if err := d.unmarshal(v.Index(n), start, depth+1); err != nil {
+ if err := d.unmarshal(v.Index(n), start); err != nil {
v.SetLen(n)
return err
}
@@ -549,15 +551,13 @@ switch t := tok.(type) {
case StartElement:
consumed := false
if sv.IsValid() {
- // unmarshalPath can call unmarshal, so we need to pass the depth through so that
- // we can continue to enforce the maximum recursion limit.
- consumed, err = d.unmarshalPath(tinfo, sv, nil, &t, depth)
+ consumed, err = d.unmarshalPath(tinfo, sv, nil, &t)
if err != nil {
return err
}
if !consumed && saveAny.IsValid() {
consumed = true
- if err := d.unmarshal(saveAny, &t, depth+1); err != nil {
+ if err := d.unmarshal(saveAny, &t); err != nil {
return err
}
}
@@ -706,7 +706,7 @@ // paths, and calls unmarshal on them.
// The consumed result tells whether XML elements have been consumed
// from the Decoder until start's matching end element, or if it's
// still untouched because start is uninteresting for sv's fields.
-func (d *Decoder) unmarshalPath(tinfo *typeInfo, sv reflect.Value, parents []string, start *StartElement, depth int) (consumed bool, err error) {
+func (d *Decoder) unmarshalPath(tinfo *typeInfo, sv reflect.Value, parents []string, start *StartElement) (consumed bool, err error) {
recurse := false
Loop:
for i := range tinfo.fields {
@@ -721,7 +721,7 @@ }
}
if len(finfo.parents) == len(parents) && finfo.name == start.Name.Local {
// It's a perfect match, unmarshal the field.
- return true, d.unmarshal(finfo.value(sv, initNilPointers), start, depth+1)
+ return true, d.unmarshal(finfo.value(sv, initNilPointers), start)
}
if len(finfo.parents) > len(parents) && finfo.parents[len(parents)] == start.Name.Local {
// It's a prefix for the field. Break and recurse
@@ -750,9 +750,7 @@ return true, err
}
switch t := tok.(type) {
case StartElement:
- // the recursion depth of unmarshalPath is limited to the path length specified
- // by the struct field tag, so we don't increment the depth here.
- consumed2, err := d.unmarshalPath(tinfo, sv, parents, &t, depth)
+ consumed2, err := d.unmarshalPath(tinfo, sv, parents, &t)
if err != nil {
return true, err
}
diff --git a/src/encoding/xml/read_test.go b/src/encoding/xml/read_test.go
index f0f7b31ccc73ff256d206468ad2412d015e9dadd..0a5bd58b7f8605bf46200398a57f95e0eefd1859 100644
--- a/src/encoding/xml/read_test.go
+++ b/src/encoding/xml/read_test.go
@@ -8,6 +8,7 @@ import (
"bytes"
"errors"
"io"
+ "os"
"reflect"
"runtime"
"strings"
@@ -1126,3 +1127,156 @@ Things []string
}
Unmarshal(bytes.Repeat([]byte(""), 17_000_000), &example)
}
+
+type recursiveNode struct {
+ XMLName Name
+ Children []recursiveNode `xml:",any"`
+}
+
+func (n *recursiveNode) UnmarshalXML(d *Decoder, start StartElement) error {
+ type alias recursiveNode
+ var a alias
+ if err := d.DecodeElement(&a, &start); err != nil {
+ return err
+ }
+ *n = recursiveNode(a)
+ return nil
+}
+
+func TestDecodeElementRecursion(t *testing.T) {
+ // The wazero builder is unable to build the test binary due to its small
+ // stack size.
+ builder := os.Getenv("GO_BUILDER_NAME")
+ if testing.Short() || strings.Contains(builder, "wazero") {
+ t.Skip("test requires significant memory")
+ }
+ maxDepth := maxUnmarshalDepth
+ if runtime.GOARCH == "wasm" {
+ maxDepth = maxUnmarshalDepthWasm
+ }
+ tests := []struct {
+ name string
+ depth int
+ wantErr error
+ }{
+ {
+ name: "below limit",
+ depth: maxDepth,
+ wantErr: nil,
+ },
+ {
+ name: "above limit",
+ depth: maxDepth + 1,
+ wantErr: errUnmarshalDepth,
+ },
+ }
+
+ for _, tt := range tests {
+ t.Run(tt.name, func(t *testing.T) {
+ payload := bytes.Join([][]byte{
+ bytes.Repeat([]byte(""), tt.depth),
+ bytes.Repeat([]byte(""), tt.depth),
+ }, nil)
+ var n recursiveNode
+ err := Unmarshal(payload, &n)
+ if err != tt.wantErr {
+ t.Fatalf("unexpected error: got %v, want %v", err, tt.wantErr)
+ }
+ })
+ }
+}
+
+type standardNode struct {
+ Sub *standardNode `xml:"section"`
+ Custom *customUnmarshalerNode `xml:"extension"`
+}
+
+type customUnmarshalerNode struct {
+ Body standardNode
+}
+
+func (e *customUnmarshalerNode) UnmarshalXML(d *Decoder, start StartElement) error {
+ var body standardNode
+ if err := d.DecodeElement(&body, &start); err != nil {
+ return err
+ }
+ e.Body = body
+ return nil
+}
+
+func TestDecodeElementDepthBypass(t *testing.T) {
+ // Construct a document with 3 blocks of 5,000 nested tags,
+ // separated by tags.
+ // Total XML nesting depth = 15,003 tags deep (maxUnmarshalDepth is 10,000).
+ openSections := strings.Repeat("", 5000)
+ closeSections := strings.Repeat("", 5000)
+
+ var buf bytes.Buffer
+ for range 3 {
+ buf.WriteString(openSections)
+ buf.WriteString("")
+ }
+ for range 3 {
+ buf.WriteString("")
+ buf.WriteString(closeSections)
+ }
+
+ var node standardNode
+ err := Unmarshal(buf.Bytes(), &node)
+
+ if err != errUnmarshalDepth {
+ t.Fatalf("Unexpected error: got %q want %q", err, errUnmarshalDepth)
+ }
+}
+
+type manualNode struct {
+ Child *manualNode
+}
+
+func (m *manualNode) UnmarshalXML(d *Decoder, start StartElement) error {
+ for {
+ tok, err := d.Token()
+ if err != nil {
+ return err
+ }
+ switch t := tok.(type) {
+ case StartElement:
+ var child manualNode
+ if err := d.DecodeElement(&child, &t); err != nil {
+ return err
+ }
+ m.Child = &child
+ case EndElement:
+ return nil
+ }
+ }
+}
+
+func TestRecursiveUnmarshalInterfaceDepth(t *testing.T) {
+ depth := maxUnmarshalDepth + 1
+ payload := bytes.Join([][]byte{
+ bytes.Repeat([]byte(""), depth),
+ bytes.Repeat([]byte(""), depth),
+ }, nil)
+
+ var node manualNode
+ err := Unmarshal(payload, &node)
+ if err != errUnmarshalDepth {
+ t.Fatalf("Unexpected error: got %q want %q", err, errUnmarshalDepth)
+ }
+}
+
+type rawTokenNode struct{}
+
+func (r *rawTokenNode) UnmarshalXML(d *Decoder, start StartElement) error {
+ _, err := d.RawToken()
+ return err
+}
+
+func TestUnmarshalXMLRawToken(t *testing.T) {
+ var node rawTokenNode
+ err := Unmarshal([]byte(""), &node)
+ if err != errRawToken {
+ t.Fatalf("UnmarshalXML calling RawToken: got error %v, want %v", err, errRawToken)
+ }
+}
diff --git a/src/encoding/xml/xml.go b/src/encoding/xml/xml.go
index 951676d4032fe4244b4c85c187030aefa3808662..455dcce5fc1f3231fa333f5cd839d56c884528ab 100644
--- a/src/encoding/xml/xml.go
+++ b/src/encoding/xml/xml.go
@@ -202,6 +202,7 @@ t TokenReader
buf bytes.Buffer
saved *bytes.Buffer
stk *stack
+ stkDepth int
free *stack
needClose bool
toClose Name
@@ -212,7 +213,7 @@ err error
line int
linestart int64
offset int64
- unmarshalDepth int
+ inUnmarshalXML bool
}
// NewDecoder creates a new XML parser reading from r.
@@ -398,6 +399,9 @@ s = new(stack)
}
s.next = d.stk
s.kind = kind
+ if kind == stkStart {
+ d.stkDepth++
+ }
d.stk = s
return s
}
@@ -405,6 +409,9 @@
func (d *Decoder) pop() *stack {
s := d.stk
if s != nil {
+ if s.kind == stkStart {
+ d.stkDepth--
+ }
d.stk = s.next
s.next = d.free
d.free = s
@@ -542,7 +549,7 @@ // RawToken is like [Decoder.Token] but does not verify that
// start and end elements match and does not translate
// name space prefixes to their corresponding URLs.
func (d *Decoder) RawToken() (Token, error) {
- if d.unmarshalDepth > 0 {
+ if d.inUnmarshalXML {
return nil, errRawToken
}
return d.rawToken()