Skip to content

Commit 6b52e92

Browse files
jcschaffclaude
jcschaff
and
claude
committed
Add XmlChars helper for input validation against invalid XML chars
Centralizes XML 1.0 character validation rules, plus project policy hard-rejecting U+FFFD (almost always charset corruption). Two modes: name (forbids whitespace) and attribute-content (allows TAB/LF/CR). Motivated by two stored BioModels (311226221, 311875206) whose cached VCML contained C0 control chars in reaction-name attributes and could no longer be parsed. The helper itself is a defensive primitive; this commit adds only the helper + tests. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 464cb68 commit 6b52e92

2 files changed

Lines changed: 270 additions & 0 deletions

File tree

‎vcell-util/src/main/java/org/vcell/util/xml/XmlChars.java‎

Lines changed: 120 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,120 @@
1+
package org.vcell.util.xml;
2+
3+
/**
4+
* XML character validation helper. Used to keep invalid-XML chars out of
5+
* model strings before they reach a CLOB or VCML attribute. See PR drafted
6+
* after observing two stored BioModels (311226221, 311875206) whose cached
7+
* VCML contained C0 control characters in reaction-name attributes and
8+
* therefore failed to load.
9+
*
10+
* <p>What we forbid (in addition to XML 1.0's own rules):
11+
* <ul>
12+
* <li>{@code U+FFFD REPLACEMENT CHARACTER} — almost always evidence of
13+
* upstream charset corruption, never legitimate in a model identifier
14+
* or attribute.</li>
15+
* </ul>
16+
*
17+
* <p>Two contexts are distinguished:
18+
* <ul>
19+
* <li><b>Name mode</b> — for entity names ({@code Reaction.name},
20+
* {@code Species.name}, etc.). Forbids whitespace as well.</li>
21+
* <li><b>Attribute-content mode</b> — for general XML attribute values.
22+
* Allows TAB/LF/CR.</li>
23+
* </ul>
24+
*
25+
* <p>All methods are stateless and thread-safe.
26+
*/
27+
public final class XmlChars {
28+
29+
private XmlChars() {}
30+
31+
/**
32+
* @param cp Unicode codepoint
33+
* @return true if {@code cp} is allowed inside an XML 1.0 attribute value
34+
* (and additionally not {@code U+FFFD}, not an unpaired surrogate,
35+
* not a non-character codepoint).
36+
*/
37+
public static boolean isValidXml10Char(int cp) {
38+
if (cp < 0x20) {
39+
return cp == 0x09 || cp == 0x0A || cp == 0x0D;
40+
}
41+
if (cp >= 0xD800 && cp <= 0xDFFF) return false; // unpaired surrogates
42+
if (cp == 0xFFFD) return false; // replacement char (project policy)
43+
if (cp >= 0xFDD0 && cp <= 0xFDEF) return false; // non-characters
44+
if ((cp & 0xFFFE) == 0xFFFE) return false; // U+nFFFE / U+nFFFF
45+
return cp <= 0x10FFFF;
46+
}
47+
48+
/**
49+
* Same as {@link #isValidXml10Char(int)} but additionally forbids any
50+
* whitespace codepoint. Use for entity names.
51+
*/
52+
public static boolean isValidNameChar(int cp) {
53+
if (!isValidXml10Char(cp)) return false;
54+
return !Character.isWhitespace(cp);
55+
}
56+
57+
/**
58+
* Scans {@code s} for the first invalid char.
59+
*
60+
* @param nameMode if true, applies {@link #isValidNameChar(int)};
61+
* else applies {@link #isValidXml10Char(int)}.
62+
* @return the {@code char} index (UTF-16 unit) of the first invalid
63+
* char, or {@code -1} if none.
64+
*/
65+
public static int firstInvalidIndex(CharSequence s, boolean nameMode) {
66+
if (s == null) return -1;
67+
int i = 0;
68+
while (i < s.length()) {
69+
int cp = Character.codePointAt(s, i);
70+
boolean ok = nameMode ? isValidNameChar(cp) : isValidXml10Char(cp);
71+
if (!ok) return i;
72+
i += Character.charCount(cp);
73+
}
74+
return -1;
75+
}
76+
77+
/**
78+
* Validates {@code value} as an entity name. Throws if any char is
79+
* forbidden. The exception message identifies the field, the offset,
80+
* the offending codepoint, and a short snippet for context.
81+
*/
82+
public static void requireValidName(String value, String fieldDesc) {
83+
require(value, fieldDesc, true);
84+
}
85+
86+
/**
87+
* Validates {@code value} as XML attribute content. Throws if any char
88+
* is forbidden.
89+
*/
90+
public static void requireValidAttributeContent(String value, String fieldDesc) {
91+
require(value, fieldDesc, false);
92+
}
93+
94+
private static void require(String value, String fieldDesc, boolean nameMode) {
95+
if (value == null) return;
96+
int idx = firstInvalidIndex(value, nameMode);
97+
if (idx < 0) return;
98+
int cp = Character.codePointAt(value, idx);
99+
throw new IllegalArgumentException(format(value, fieldDesc, idx, cp));
100+
}
101+
102+
private static String format(String value, String fieldDesc, int idx, int cp) {
103+
StringBuilder snippet = new StringBuilder();
104+
int from = Math.max(0, idx - 10);
105+
int to = Math.min(value.length(), idx + 10);
106+
for (int i = from; i < to; i++) {
107+
char c = value.charAt(i);
108+
if (c < 0x20 && c != 0x09) {
109+
snippet.append(String.format("\\x%02x", (int) c));
110+
} else if (c == 0xFFFD) {
111+
snippet.append("<U+FFFD>");
112+
} else {
113+
snippet.append(c);
114+
}
115+
}
116+
return String.format(
117+
"invalid character in %s at index %d (codepoint 0x%04X): \"%s\"",
118+
fieldDesc, idx, cp, snippet.toString());
119+
}
120+
}

‎vcell-util/src/test/java/org/vcell/util/xml/XmlCharsTest.java‎

Lines changed: 150 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,150 @@
1+
package org.vcell.util.xml;
2+
3+
import org.junit.jupiter.api.Tag;
4+
import org.junit.jupiter.api.Test;
5+
6+
import static org.junit.jupiter.api.Assertions.assertEquals;
7+
import static org.junit.jupiter.api.Assertions.assertFalse;
8+
import static org.junit.jupiter.api.Assertions.assertNotNull;
9+
import static org.junit.jupiter.api.Assertions.assertThrows;
10+
import static org.junit.jupiter.api.Assertions.assertTrue;
11+
12+
@Tag("Fast")
13+
public class XmlCharsTest {
14+
15+
@Test
16+
public void plainAscii_isValid() {
17+
assertEquals(-1, XmlChars.firstInvalidIndex("Reaction_1", true));
18+
assertEquals(-1, XmlChars.firstInvalidIndex("Reaction_1", false));
19+
}
20+
21+
@Test
22+
public void unicodeLetters_areValid() {
23+
// greek mu, en-dash; legitimate in scientific names
24+
assertEquals(-1, XmlChars.firstInvalidIndex("μ-prot", true));
25+
assertEquals(-1, XmlChars.firstInvalidIndex("k_14–3–3", true));
26+
}
27+
28+
@Test
29+
public void supplementaryPlane_isValid() {
30+
// U+1F600 (surrogate pair) — not whitespace, valid in name mode
31+
String emoji = new String(Character.toChars(0x1F600));
32+
assertEquals(-1, XmlChars.firstInvalidIndex(emoji, true));
33+
}
34+
35+
@Test
36+
public void tabLfCr_validInAttributes_invalidInNames() {
37+
assertEquals(-1, XmlChars.firstInvalidIndex("a\tb", false));
38+
assertEquals(-1, XmlChars.firstInvalidIndex("a\nb", false));
39+
assertEquals(-1, XmlChars.firstInvalidIndex("a\rb", false));
40+
assertEquals(1, XmlChars.firstInvalidIndex("a\tb", true));
41+
assertEquals(1, XmlChars.firstInvalidIndex("a\nb", true));
42+
assertEquals(1, XmlChars.firstInvalidIndex("a\rb", true));
43+
}
44+
45+
@Test
46+
public void spaceRejectedInName_acceptedInAttribute() {
47+
assertEquals(-1, XmlChars.firstInvalidIndex("a b", false));
48+
assertEquals(1, XmlChars.firstInvalidIndex("a b", true));
49+
}
50+
51+
@Test
52+
public void controlChar_0x13_rejected() {
53+
// observed in biomodel 311226221
54+
String bad = "k_reid_3";
55+
assertEquals(4, XmlChars.firstInvalidIndex(bad, true));
56+
assertEquals(4, XmlChars.firstInvalidIndex(bad, false));
57+
}
58+
59+
@Test
60+
public void controlChar_0x1C_rejected() {
61+
// observed in biomodel 311875206 (paired with U+FFFD)
62+
String bad = "namesuffix";
63+
assertEquals(4, XmlChars.firstInvalidIndex(bad, true));
64+
}
65+
66+
@Test
67+
public void replacementChar_rejectedByPolicy() {
68+
// U+FFFD — XML 1.0 technically allows it; project policy rejects
69+
String bad = "name�suffix";
70+
assertFalse(XmlChars.isValidXml10Char(0xFFFD));
71+
assertEquals(4, XmlChars.firstInvalidIndex(bad, true));
72+
assertEquals(4, XmlChars.firstInvalidIndex(bad, false));
73+
}
74+
75+
@Test
76+
public void realWorldPattern_replacementPlus0x1C_rejected() {
77+
// exact pattern observed in cached VCML of biomodel 311875206
78+
String bad = "rxn�end";
79+
// U+FFFD comes first
80+
assertEquals(3, XmlChars.firstInvalidIndex(bad, true));
81+
}
82+
83+
@Test
84+
public void unpairedSurrogate_rejected() {
85+
assertFalse(XmlChars.isValidXml10Char(0xD800));
86+
assertFalse(XmlChars.isValidXml10Char(0xDFFF));
87+
// A lone high surrogate as a single char
88+
String lone = "x" + (char) 0xD800 + "y";
89+
assertEquals(1, XmlChars.firstInvalidIndex(lone, false));
90+
}
91+
92+
@Test
93+
public void nonCharacterCodepoints_rejected() {
94+
assertFalse(XmlChars.isValidXml10Char(0xFDD0));
95+
assertFalse(XmlChars.isValidXml10Char(0xFDEF));
96+
assertFalse(XmlChars.isValidXml10Char(0xFFFE));
97+
assertFalse(XmlChars.isValidXml10Char(0xFFFF));
98+
assertFalse(XmlChars.isValidXml10Char(0x1FFFE));
99+
assertFalse(XmlChars.isValidXml10Char(0x10FFFF));
100+
}
101+
102+
@Test
103+
public void nullInput_isHandled() {
104+
assertEquals(-1, XmlChars.firstInvalidIndex(null, true));
105+
XmlChars.requireValidName(null, "any");
106+
XmlChars.requireValidAttributeContent(null, "any");
107+
}
108+
109+
@Test
110+
public void emptyString_isValid() {
111+
assertEquals(-1, XmlChars.firstInvalidIndex("", true));
112+
XmlChars.requireValidName("", "any");
113+
}
114+
115+
@Test
116+
public void requireValidName_throwsWithFieldOffsetCodepoint() {
117+
IllegalArgumentException ex = assertThrows(
118+
IllegalArgumentException.class,
119+
() -> XmlChars.requireValidName("rxnend", "Reaction.name"));
120+
String msg = ex.getMessage();
121+
assertNotNull(msg);
122+
assertTrue(msg.contains("Reaction.name"), msg);
123+
assertTrue(msg.contains("index 3"), msg);
124+
assertTrue(msg.contains("0x0013"), msg);
125+
assertTrue(msg.contains("\\x13"), msg);
126+
}
127+
128+
@Test
129+
public void requireValidAttributeContent_acceptsTabAndNewline() {
130+
XmlChars.requireValidAttributeContent("a\tb\nc", "some.attr");
131+
}
132+
133+
@Test
134+
public void requireValidAttributeContent_rejectsReplacementChar() {
135+
IllegalArgumentException ex = assertThrows(
136+
IllegalArgumentException.class,
137+
() -> XmlChars.requireValidAttributeContent("a�b", "some.attr"));
138+
assertTrue(ex.getMessage().contains("0xFFFD"), ex.getMessage());
139+
assertTrue(ex.getMessage().contains("<U+FFFD>"), ex.getMessage());
140+
}
141+
142+
@Test
143+
public void firstInvalidIndex_returnsUtf16Index_notCodepointIndex() {
144+
// surrogate pair (length 2) followed by bad char at char index 2
145+
String emoji = new String(Character.toChars(0x1F600));
146+
String s = emoji + "";
147+
assertEquals(emoji.length(), XmlChars.firstInvalidIndex(s, false));
148+
assertEquals(2, XmlChars.firstInvalidIndex(s, false));
149+
}
150+
}

0 commit comments

Comments
 (0)