Skip to content

Escape TAB/LF/CR again when serializing to XML - #1093

Open
Lyin-OTTF wants to merge 1 commit into
stleary:masterfrom
Lyin-OTTF:fix/xml-serialize-escape-whitespace
Open

Lyin-OTTF wants to merge 1 commit into
stleary:masterfrom
Lyin-OTTF:fix/xml-serialize-escape-whitespace

Conversation

@Lyin-OTTF

Copy link
Copy Markdown

Description

Fixes #1092.

XML.mustEscape() is shared by two callers that need opposite answers for TAB (0x9), LF (0xA) and CR (0xD):

  • XML.escape() uses it to decide what to escape when serializing.
  • XMLTokener.unescapeEntity() uses it to reject invalid numeric character references when parsing.

#1070 added those three codepoints to the valid-range clause so that unescape("
") works again (#1059). A side effect was that the serialization path stopped escaping literal TAB/LF/CR: XML.toString() and JSONML.toString() now write them as literal characters. Conforming XML parsers normalize whitespace, so a value like "Lettuce\nTomato" loses its line breaks when the generated XML is read back by any standard parser — the attribute value comes back as "Lettuce Tomato" (or with collapsed whitespace), and XML.toJSONObject(XML.toString(o)) does not return the original string.

This PR splits the two concerns:

Tests

Three new tests in XMLTest:

  • testEscapesLiteralWhitespaceControlCharsOnSerialization — XML.toString() writes 
/	/
 for literal TAB/LF/CR. Fails on master.
  • testWhitespaceControlCharsRoundTripThroughXml — XML.toJSONObject(XML.toString(o)) returns the original value with the line break intact.
  • testJSONMLToStringEscapesLiteralNewlines — same requirement for JSONML.toString().
mvn test
Tests run: 827, Failures: 0, Errors: 0, Skipped: 6

The existing #1059/#1070 regression tests (testValidWhitespaceNumericEntityUnescape, testValidWhitespaceNumericEntityToJSONObject) still pass.

XML.mustEscape() is shared by XML.escape() (serialization) and
XMLTokener.unescapeEntity() (parsing), but the two callers need opposite
answers for #x9/#xA/#xD. stleary#1070 added those codepoints to the valid-range
clause to fix unescape("&stleary#10;"), which also stopped XML.toString() and
JSONML.toString() from escaping literal TAB, LF and CR. Literal
occurrences of these characters are subject to whitespace normalization
by conforming XML parsers, so a value like "Lettuce\nTomato" loses its
line breaks when the generated XML is read back by any standard parser.
This regressed stleary#1092.

Split the two concerns: mustEscape() returns true for every ISO control
character again (serialization), and the new isValidXmlCharacter()
checks the XML 1.0 character range for numeric character references
during parsing. unescapeEntity() now validates with
isValidXmlCharacter(), so &stleary#9;, &stleary#10; and &stleary#13; keep being accepted
(stleary#1059/stleary#1070 behavior unchanged).

Fixes stleary#1092
@stleary

stleary commented Oct 20, 2026

Copy link
Copy Markdown
Owner

@Lyin-OTTF No files changed?

@Lyin-OTTF
Lyin-OTTF force-pushed the fix/xml-serialize-escape-whitespace branch from 1cf10b5 to 3b93afd Compare October 21, 2026 01:23
@sonarqubecloud

Copy link
Copy Markdown

@Lyin-OTTF

Copy link
Copy Markdown
Author

The first push came out empty. I built the tree with the Git Data API and reused the parent tree, so the changed files never made it into the commit - the tree was identical to master, which is what you saw.

Force-pushed the same commit with the actual changes. The diff is there now: 3 files, +80/-20.

src/main/java/org/json/XML.java           | 44 ++++++++-------
src/main/java/org/json/XMLTokener.java    |  6 +++-
src/test/java/org/json/junit/XMLTest.java | 50 +++++++++++++++

Tests: 827 run, 0 failures, 0 errors, 6 skipped. The three new tests in XMLTest fail without the main-source change and pass with it.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

JSONML.toString() and XML.toString() omit escaping for literal newlines (\n), causing whitespace loss in downstream XML parsers (Regression from #1070)

2 participants