Permalink
Browse files

Issue #92 - Fix off-by-one bug introduced in character verification.

The code will now verify the second character in a name.
Additionally, the error messages for broken names has been improved to indicate what both the broken name and character are, instead of just the character.
Also, change the checkNamespacePrefix code to leverage the common code from checkElementName and checkAttributeName.

Also improve the test harness to cover test cases that are now more complicated because the name-check code is in two places now instead of one....
  • Loading branch information...
1 parent 13bef50 commit d72596e670e0823f2ff1482b12f26e6117f02030 @rolfl rolfl committed Sep 9, 2012
Showing with 28 additions and 26 deletions.
  1. +10 −23 core/src/java/org/jdom2/Verifier.java
  2. +18 −3 test/src/java/org/jdom2/test/cases/TestVerifier.java
@@ -366,14 +366,15 @@ private static final String checkJDOMName(final String name) {
// Cannot start with a number
if ((byte)0 == (CHARFLAGS[name.charAt(0)] & MASKXMLSTARTCHAR)) {
- return "XML names cannot begin with the character \"" +
+ return "XML name '" + name + "' cannot begin with the character \"" +
name.charAt(0) + "\"";
}
// Ensure legal content for non-first chars
// also check char 0 to catch colon char ':'
- for (int i = name.length() - 1; i > 1; i--) {
+ for (int i = name.length() - 1; i >= 1; i--) {
rolfl
rolfl Sep 9, 2012 Collaborator

Off-by-one bug here:

if ((byte)0 == (byte)(CHARFLAGS[name.charAt(i)] & MASKXMLNAMECHAR)) {
- return "XML names cannot contain the character \"" + name.charAt(i) + "\"";
+ return "XML name '" + name + "' cannot contain the character \""
+ + name.charAt(i) + "\"";
}
}
@@ -531,22 +532,14 @@ public static String checkNamespacePrefix(final String prefix) {
return null;
}
- final int len = prefix.length();
-
- // Cannot start with a number
- if ((byte)0 == (byte)(CHARFLAGS[prefix.charAt(0)] & MASKXMLSTARTCHAR)) {
- return "Namespace prefixes cannot begin with character '" + prefix.charAt(0) + "'";
- }
- // Ensure legal content
- for (int i=1; i < len; i++) {
- if ((byte)0 == (byte)(CHARFLAGS[prefix.charAt(i)] & MASKXMLNAMECHAR)) {
- return "Namespace prefixes cannot contain the character \"" +
- prefix.charAt(i) + "\"";
- }
+ if (checkJDOMName(prefix) != null) {
+ // will double-check null and empty names, but that's OK
+ // since we have already checked them.
+ return checkJDOMName(prefix);
}
-
+
// Cannot start with "xml" in any character case
- if (len >= 3) {
+ if (prefix.length() >= 3) {
if (prefix.charAt(0) == 'x' || prefix.charAt(0) == 'X') {
if (prefix.charAt(1) == 'm' || prefix.charAt(1) == 'M') {
if (prefix.charAt(2) == 'l' || prefix.charAt(2) == 'L') {
@@ -557,12 +550,6 @@ public static String checkNamespacePrefix(final String prefix) {
}
}
-
- // No colons allowed
- if (prefix.indexOf(":") != -1) {
- return "Namespace prefixes cannot contain colons";
- }
-
// If we got here, everything is OK
return null;
}
@@ -99,14 +99,19 @@ public static void main (String args[])
public void testCheckElementName() {
//check out of range values
assertNotNull("validated invalid null", Verifier.checkElementName(null));
- assertNotNull("validated invalid name with null", Verifier.checkElementName("test" + (char)0x0));
- assertNotNull("validated invalid name with null", Verifier.checkElementName("test" + (char)0x0 + "ing"));
- assertNotNull("validated invalid name with null", Verifier.checkElementName((char)0x0 + "test"));
+ assertNotNull("validated invalid name with null char", Verifier.checkElementName("test" + (char)0x0));
+ assertNotNull("validated invalid name with null char", Verifier.checkElementName("test" + (char)0x0 + "ing"));
+ assertNotNull("validated invalid name with null char", Verifier.checkElementName((char)0x0 + "test"));
assertNotNull("validated invalid name with 0x01", Verifier.checkElementName((char)0x01 + "test"));
assertNotNull("validated invalid name with 0xD800", Verifier.checkElementName("test" + (char)0xD800));
assertNotNull("validated invalid name with 0xD800", Verifier.checkElementName("test" + (char)0xD800 + "ing"));
assertNotNull("validated invalid name with 0xD800", Verifier.checkElementName((char)0xD800 + "test"));
assertNotNull("validated invalid name with :", Verifier.checkElementName("test" + ':' + "local"));
+ assertNotNull("validated invalid name with :", Verifier.checkElementName("abcd:"));
+ assertNotNull("validated invalid name with :", Verifier.checkElementName("abc:d"));
+ assertNotNull("validated invalid name with :", Verifier.checkElementName("ab:cd"));
+ assertNotNull("validated invalid name with :", Verifier.checkElementName("a:bcd"));
+ assertNotNull("validated invalid name with :", Verifier.checkElementName(":abcd"));
//invalid start characters
assertNotNull("validated invalid name with startin -", Verifier.checkElementName('-' + "test"));
@@ -256,16 +261,26 @@ public void testCheckNamespacePrefix() {
assertNotNull("validated invalid name with starting digit", Verifier.checkNamespacePrefix("9"));
assertNotNull("validated invalid name with starting $", Verifier.checkNamespacePrefix("$"));
assertNotNull("validated invalid name with starting .", Verifier.checkNamespacePrefix("."));
+
+ // cannot start with xml (case insensitive).
+ assertNotNull("validated invalid name beginning with xml", Verifier.checkNamespacePrefix("xmlabc"));
+ assertNotNull("validated invalid name beginning with xml", Verifier.checkNamespacePrefix("xmLabc"));
+ assertNotNull("validated invalid name beginning with xml", Verifier.checkNamespacePrefix("xMlabc"));
+ assertNotNull("validated invalid name beginning with xml", Verifier.checkNamespacePrefix("Xmlabc"));
+
//valid tests
assertNull("invalidated valid null", Verifier.checkNamespacePrefix(null));
+ assertNull("invalidated valid empty String", Verifier.checkNamespacePrefix(""));
assertNull("invalidated valid name with starting _", Verifier.checkNamespacePrefix('_' + "test"));
assertNull("invalidated valid name with _", Verifier.checkNamespacePrefix("test" + '_'));
assertNull("invalidated valid name with .", Verifier.checkNamespacePrefix("test" + '.' + "name"));
assertNull("invalidated valid name with digit", Verifier.checkNamespacePrefix("test9"));
assertNull("invalidated valid name with 0x00B7", Verifier.checkNamespacePrefix("test" + (char)0x00B7));
assertNull("invalidated valid name with 0x4E01", Verifier.checkNamespacePrefix("test" + (char)0x4E01));
assertNull("invalidated valid name with 0x0301", Verifier.checkNamespacePrefix("test" + (char)0x0301));
+ assertNull("invalidated valid name with xml embedded", Verifier.checkNamespacePrefix("txml"));
+ assertNull("invalidated valid name with xml embedded", Verifier.checkNamespacePrefix("xmml"));
}

0 comments on commit d72596e

Please sign in to comment.