Escape TAB/LF/CR again when serializing to XML

XML.mustEscape() is shared by XML.escape() (serialization) and
XMLTokener.unescapeEntity() (parsing), but the two callers need opposite
answers for #x9/#xA/#xD. #1070 added those codepoints to the valid-range
clause to fix unescape("
"), 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 #1092.

Split the two concerns: mustEscape() returns true for every ISO control
character again (serialization), and the new isValidXmlCharacter() keeps
the previous numeric-reference validation for the parsing path, so 	,

 and 
 are still accepted (#1059/#1070 behaviour unchanged).

isValidXmlCharacter() stays as strict as before for the C1 control
characters (#x7F-#x9F): they fall inside [#x20-#xD7FF] and are legal XML
1.0 characters, but this library has always rejected references to them,
and widening the range here would silently change what XML.toJSONObject
accepts. Only the serialization of TAB/LF/CR changes.

Fixes #1092

Author: Lyin-OTTF

src/main/java/org/json/XML.java

@@ -156,34 +156,45 @@ public static String escape(String string) {
 
     /**
      * @param cp code point to test
-     * @return true if the code point is not valid for an XML
+     * @return true if the code point must be escaped when serializing to XML
      */
     static boolean mustEscape(int cp) {
+        // Every ISO control character must be written as a numeric character
+        // reference when serializing. This includes #x9 (tab), #xA (LF) and
+        // #xD (CR): literal occurrences of these characters are subject to
+        // whitespace normalization by conforming XML parsers, so they can
+        // only survive a round trip as 	, 
 and 
 (see #1092).
+        //
+        // This method is only about the serialization path. Numeric character
+        // references found while parsing are validated with
+        // isValidXmlCharacter(int) (see #1059).
+        return Character.isISOControl(cp) || !isValidXmlCharacter(cp);
+    }
+
+    /**
+     * @param cp code point to test
+     * @return true if the code point can be written as a numeric character
+     *         reference in a document generated by this class
+     */
+    static boolean isValidXmlCharacter(int cp) {
         /* Valid range from https://www.w3.org/TR/REC-xml/#charsets
          *
          * #x9 | #xA | #xD | [#x20-#xD7FF] | [#xE000-#xFFFD] | [#x10000-#x10FFFF]
          *
          * any Unicode character, excluding the surrogate blocks, FFFE, and FFFF.
          */
-        // isISOControl is true when (cp >= 0 && cp <= 0x1F) || (cp >= 0x7F && cp <= 0x9F)
-        // all ISO control characters are out of range except tabs and new lines
-        return (Character.isISOControl(cp)
-                && cp != 0x9
-                && cp != 0xA
-                && cp != 0xD
-            ) || !(
-                // Valid character range per W3C XML 1.0 spec:
-                // #x9 | #xA | #xD | [#x20-#xD7FF] | [#xE000-#xFFFD] | [#x10000-#x10FFFF]
-                // Previously omitted #x9/#xA/#xD, causing unescape("&#10;") etc.
-                // to reject valid LF/TAB/CR as illegal (see #1059)
-                cp == 0x9
-                || cp == 0xA
-                || cp == 0xD
-                || (cp >= 0x20 && cp <= 0xD7FF)
-                || (cp >= 0xE000 && cp <= 0xFFFD)
-                || (cp >= 0x10000 && cp <= 0x10FFFF)
-            )
-        ;
+        // The C1 control characters (#x7F-#x9F) fall inside [#x20-#xD7FF] and
+        // are therefore legal XML 1.0 characters, but this class has always
+        // rejected &#NN; references to them while parsing. Keeping them
+        // rejected here keeps the parse result of this fix identical to the
+        // previous release; accepting them would be a separate behaviour
+        // change (see #1093).
+        if (Character.isISOControl(cp)) {
+            return cp == 0x9 || cp == 0xA || cp == 0xD;
+        }
+        return (cp >= 0x20 && cp <= 0xD7FF)
+            || (cp >= 0xE000 && cp <= 0xFFFD)
+            || (cp >= 0x10000 && cp <= 0x10FFFF);
     }
 
     /**

src/main/java/org/json/XMLTokener.java

@@ -167,7 +167,11 @@ static String unescapeEntity(String e) throws JSONException {
             int cp = (e.charAt(1) == 'x' || e.charAt(1) == 'X')
                 ? parseHexEntity(e)
                 : parseDecimalEntity(e);
-            if (XML.mustEscape(cp)) {
+            // Validate with isValidXmlCharacter, not with XML.mustEscape:
+            // mustEscape also escapes TAB/LF/CR for the serialization path
+            // (#1092), while the numeric references 	, 
 and 
 are
+            // valid XML and must keep being accepted here (see #1059).
+            if (!XML.isValidXmlCharacter(cp)) {
                 throw new JSONException("Invalid numeric character reference: &#" + e.substring(1) + ";");
             }
             return new String(new int[] {cp}, 0, 1);

src/test/java/org/json/junit/XMLTest.java

@@ -7,6 +7,7 @@
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotEquals;
+import static org.junit.Assert.assertThrows;
 import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
 
@@ -1610,6 +1611,76 @@ public void testValidWhitespaceNumericEntityToJSONObject() {
         XML.toJSONObject("<a>&#13;</a>");
     }
 
+    /**
+     * Tests that numeric character references to C1 control characters are
+     * still rejected. #x7F-#x9F is inside the XML 1.0 {@code Char} range, but
+     * this library has always refused those references while parsing
+     * ({@code &#133;} is NEL, #x85), and fixing the serialization of TAB/LF/CR
+     * must not change what the parser accepts.
+     */
+    @Test
+    public void testRejectsC1ControlCharacterReference() {
+        assertThrows(JSONException.class,
+                () -> XML.toJSONObject("<name>Joe &#133; Tester</name>"));
+        // the neighbouring boundary codepoints behave the same way
+        assertThrows(JSONException.class,
+                () -> XML.toJSONObject("<name>Joe &#127; Tester</name>"));
+        assertThrows(JSONException.class,
+                () -> XML.toJSONObject("<name>Joe &#159; Tester</name>"));
+        // ... while the characters the parser does accept still work
+        XML.toJSONObject("<name>Joe &#32; Tester</name>");
+    }
+
+    /**
+     * Tests that serializing a string value containing literal TAB, LF and CR
+     * characters writes numeric character references (&amp;#9;, &amp;#10;,
+     * &amp;#13;) instead of the literal characters. Literal occurrences of
+     * these characters are subject to whitespace normalization by conforming
+     * XML parsers, so the original value would be lost when the generated XML
+     * is read back. Regression test for #1092, where #1070 stopped
+     * XML.toString() and JSONML.toString() from escaping these characters.
+     */
+    @Test
+    public void testEscapesLiteralWhitespaceControlCharsOnSerialization() {
+        // LF in a text node must survive as a numeric character reference
+        JSONObject jo = new JSONObject().put("value", "Lettuce\nTomato");
+        String xmlOutput = XML.toString(jo);
+        assertTrue("XML should escape literal LF to preserve it, got: " + xmlOutput,
+                xmlOutput.contains("&#xa;"));
+
+        // TAB and CR likewise
+        JSONObject tabJo = new JSONObject().put("value", "a\tb\rc");
+        String tabXml = XML.toString(tabJo);
+        assertTrue("XML should escape literal TAB, got: " + tabXml, tabXml.contains("&#x9;"));
+        assertTrue("XML should escape literal CR, got: " + tabXml, tabXml.contains("&#xd;"));
+    }
+
+    /**
+     * Tests that the generated XML round-trips: the numeric character
+     * references produced for TAB/LF/CR are read back by
+     * {@code XML.toJSONObject} as the original characters.
+     */
+    @Test
+    public void testWhitespaceControlCharsRoundTripThroughXml() {
+        String original = "Lettuce\nTomato\tMustard";
+        JSONObject jo = new JSONObject().put("value", original);
+        String xmlOutput = XML.toString(jo);
+        JSONObject reparsed = XML.toJSONObject(xmlOutput);
+        assertEquals("LF/TAB must survive the XML round trip",
+                original, reparsed.getString("value"));
+    }
+
+    /**
+     * Same escaping requirement for {@code JSONML.toString}.
+     */
+    @Test
+    public void testJSONMLToStringEscapesLiteralNewlines() {
+        JSONObject jo = new JSONObject("{\"tagName\":\"MyTag\", \"value\":\"Lettuce\\nTomato\"}");
+        String xmlOutput = JSONML.toString(jo);
+        assertTrue("JSONML should escape literal LF to preserve it, got: " + xmlOutput,
+                xmlOutput.contains("&#xa;"));
+    }
+
 }