Skip to content

enforce maxStringLength/maxNameLength read constraints in XML parser - #917

Open
Sahana2524 wants to merge 1 commit into
FasterXML:3.xfrom
Sahana2524:xml-read-string-name-limits
Open

Sahana2524 wants to merge 1 commit into
FasterXML:3.xfrom
Sahana2524:xml-read-string-name-limits

Conversation

@Sahana2524

Copy link
Copy Markdown
Contributor

maxStringLength / maxNameLength are ignored when reading XML

The read path never runs the configured StreamReadConstraints over element text, attribute values, or element/attribute/root names, so those two limits have no effect on deserialization even though the write side and the JSON backend honor them. With maxStringLength(100) a 5000-char text value or attribute value parses cleanly, and with maxNameLength(50) a 5000-char element, attribute, or root name parses cleanly.

This complements the maxNestingDepth work in #891 by covering the content-length limits. The checks live where XmlTokenStream materializes each value: validateStringLength at the single text accessor (getText(), which every element-text and attribute value passes through) and validateNameLength at the two name-decode points (_decodeElementName / _decodeAttributeName, which every name passes through). The constraints are threaded in from IOContext. Default limits are unchanged, so normal documents behave exactly as before; maxDocumentLength (#609) is a separate concern and not addressed here.

@github-actions

Copy link
Copy Markdown

🧪 Code Coverage Report

Metric Coverage
Instructions coverage 77.76%
Branches branches 71.05%

Coverage data generated from JaCoCo test results

@cowtowncoder

Copy link
Copy Markdown
Member

This only occurs after-the-fact, XML parser having decoded element/attribute name, text value.
Not much we can do there, in general; although Woodstox does provide functionality to limit these at XML parser level.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants