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 af25c20f0618dc815daf4ab8cb49fd98abac6945..5b36ce905870da8ca2509d80c8aea3d3981d3c29 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 @@ -318,8 +320,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. @@ -415,7 +417,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 } @@ -538,15 +540,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 } } @@ -691,7 +691,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 { @@ -706,7 +706,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 @@ -735,9 +735,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()