Author: jukka
Date: Fri Mar 28 16:25:32 2014
New Revision: 1582804
URL: http://svn.apache.org/r1582804
Log:
OAK-1174: Inconsistent handling of invalid names/paths
The spec allows whitespace in names, so the parser shouldn't balk at them.
Instead we guard against them in the NameValidator during writes.
Modified:
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/namepath/JcrNameParser.java
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/namepath/JcrPathParser.java
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/name/Namespaces.java
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/namepath/NamePathMapperImplTest.java
jackrabbit/oak/trunk/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/NamePathTest.java
Modified:
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/namepath/JcrNameParser.java
URL:
http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/namepath/JcrNameParser.java?rev=1582804&r1=1582803&r2=1582804&view=diff
==============================================================================
---
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/namepath/JcrNameParser.java
(original)
+++
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/namepath/JcrNameParser.java
Fri Mar 28 16:25:32 2014
@@ -92,7 +92,6 @@ public final class JcrNameParser {
String prefix;
int nameStart = 0;
int state = STATE_PREFIX_START;
- boolean trailingSpaces = false;
for (int i = 0; i < len; i++) {
char c = jcrName.charAt(i);
@@ -101,10 +100,6 @@ public final class JcrNameParser {
listener.error("Prefix must not be empty");
return false;
} else if (state == STATE_PREFIX) {
- if (trailingSpaces) {
- listener.error("Trailing spaces not allowed");
- return false;
- }
prefix = jcrName.substring(0, i);
if (!XMLChar.isValidNCName(prefix)) {
listener.error("Invalid name prefix: "+ prefix);
@@ -117,14 +112,7 @@ public final class JcrNameParser {
listener.error("'" + c + "' not allowed in name");
return false;
}
- trailingSpaces = false;
- } else if (c == ' ') {
- if (state == STATE_PREFIX_START || state == STATE_NAME_START) {
- listener.error("'" + c + "' not valid name start");
- return false;
- }
- trailingSpaces = true;
- } else if (Character.isWhitespace(c) || c == '[' || c == ']' || c
== '*' || c == '|') {
+ } else if (c == '[' || c == ']' || c == '*' || c == '|') {
listener.error("'" + c + "' not allowed in name");
return false;
} else if (c == '/') {
@@ -134,7 +122,6 @@ public final class JcrNameParser {
listener.error("'" + c + "' not allowed in name");
return false;
}
- trailingSpaces = false;
} else if (c == '{') {
if (state == STATE_PREFIX_START) {
state = STATE_URI_START;
@@ -147,7 +134,6 @@ public final class JcrNameParser {
state = STATE_NAME;
nameStart = i;
}
- trailingSpaces = false;
} else if (c == '}') {
if (state == STATE_URI_START || state == STATE_URI) {
String tmp = jcrName.substring(1, i);
@@ -178,7 +164,6 @@ public final class JcrNameParser {
state = STATE_NAME;
nameStart = i;
}
- trailingSpaces = false;
} else {
if (state == STATE_PREFIX_START) {
state = STATE_PREFIX; // prefix start
@@ -188,7 +173,6 @@ public final class JcrNameParser {
} else if (state == STATE_URI_START) {
state = STATE_URI;
}
- trailingSpaces = false;
}
}
@@ -203,10 +187,6 @@ public final class JcrNameParser {
listener.error("Local name must not be empty");
return false;
}
- if (trailingSpaces) {
- listener.error("Trailing spaces not allowed");
- return false;
- }
return listener.name(jcrName, index);
}
Modified:
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/namepath/JcrPathParser.java
URL:
http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/namepath/JcrPathParser.java?rev=1582804&r1=1582803&r2=1582804&view=diff
==============================================================================
---
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/namepath/JcrPathParser.java
(original)
+++
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/namepath/JcrPathParser.java
Fri Mar 28 16:25:32 2014
@@ -80,11 +80,7 @@ public final class JcrPathParser {
while (pos <= len) {
char c = pos == len ? EOF : jcrPath.charAt(pos);
pos++;
- // special check for whitespace
- if (c != ' ' && Character.isWhitespace(c)) {
- c = '\t';
- }
-
+
switch (c) {
case '/':
case EOF:
@@ -205,24 +201,6 @@ public final class JcrPathParser {
}
break;
- case ' ':
- if (state == STATE_PREFIX_START || state ==
STATE_NAME_START) {
- listener.error('\'' + jcrPath + "' is not a valid
path. '" + c +
- "' not valid name start");
- return false;
- } else if (state == STATE_INDEX_END) {
- listener.error('\'' + jcrPath + "' is not a valid
path. '" + c +
- "' not valid after index. '/' expected.");
- return false;
- } else if (state == STATE_DOT || state == STATE_DOTDOT) {
- state = STATE_PREFIX;
- }
- break;
-
- case '\t':
- listener.error('\'' + jcrPath + "' is not a valid path. " +
- "Whitespace not a allowed in name.");
- return false;
case '*':
case '|':
listener.error('\'' + jcrPath + "' is not a valid path. '"
+ c +
@@ -257,7 +235,7 @@ public final class JcrPathParser {
return false;
}
}
- wasSlash = c == ' ';
+ wasSlash = c == '/';
}
return true;
}
Modified:
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/name/Namespaces.java
URL:
http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/name/Namespaces.java?rev=1582804&r1=1582803&r2=1582804&view=diff
==============================================================================
---
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/name/Namespaces.java
(original)
+++
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/name/Namespaces.java
Fri Mar 28 16:25:32 2014
@@ -244,10 +244,14 @@ public class Namespaces implements Names
for (int i = 0; i < local.length(); i++) {
char ch = local.charAt(i);
- if (i == 0 && Character.isWhitespace(ch)) {
- return false; // leading whitespace
- } else if (i == local.length() - 1 && Character.isWhitespace(ch)) {
- return false; // trailing whitespace
+ if (Character.isSpaceChar(ch)) {
+ if (i == 0) {
+ return false; // leading whitespace
+ } else if (i == local.length() - 1) {
+ return false; // trailing whitespace
+ } else if (ch != ' ') {
+ return false; // only spaces are allowed as whitespace
+ }
} else if ("/:[]|*".indexOf(ch) != -1) { // TODO: XMLChar check
return false; // invalid name character
}
Modified:
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/namepath/NamePathMapperImplTest.java
URL:
http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/namepath/NamePathMapperImplTest.java?rev=1582804&r1=1582803&r2=1582804&view=diff
==============================================================================
---
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/namepath/NamePathMapperImplTest.java
(original)
+++
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/namepath/NamePathMapperImplTest.java
Fri Mar 28 16:25:32 2014
@@ -36,14 +36,13 @@ public class NamePathMapperImplTest {
private static final Map<String, String> GLOBAL = ImmutableMap.of(
"oak-jcr", "http://www.jcp.org/jcr/1.0";,
"oak-nt", "http://www.jcp.org/jcr/nt/1.0";,
- "oak-mix", "http://www.jcp.org/jcr/mix/1.0";,
"oak-foo", "http://www.example.com/foo";,
- "oak-quu", "http://www.example.com/quu";);
+ "oak-quu", "http://www.example.com/quu";,
+ "oak", "http://jackrabbit.apache.org/oak/ns/1.0";);
private static final Map<String, String> LOCAL = ImmutableMap.of(
"jcr-jcr", "http://www.jcp.org/jcr/1.0";,
"jcr-nt", "http://www.jcp.org/jcr/nt/1.0";,
- "jcr-mix", "http://www.jcp.org/jcr/mix/1.0";,
"foo", "http://www.example.com/foo";,
"quu", "http://www.example.com/quu";);
@@ -220,4 +219,17 @@ public class NamePathMapperImplTest {
}
}
+ @Test
+ public void testWhitespace() {
+ String[] paths = new String[] {
+ " leading", "trailing\n", " ", "\t",
+ "oak: leading", "oak:trailing\n", "oak: ", "oak:\t" };
+ NamePathMapper noLocal = new NamePathMapperImpl(new LocalNameMapper(
+ GLOBAL, Collections.<String, String>emptyMap()));
+ for (String path : paths) {
+ assertEquals("without local mappings", path,
noLocal.getOakPath(path));
+ assertEquals("with local mappings", path,
npMapper.getOakPath(path));
+ }
+ }
+
}
Modified:
jackrabbit/oak/trunk/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/NamePathTest.java
URL:
http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/NamePathTest.java?rev=1582804&r1=1582803&r2=1582804&view=diff
==============================================================================
---
jackrabbit/oak/trunk/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/NamePathTest.java
(original)
+++
jackrabbit/oak/trunk/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/NamePathTest.java
Fri Mar 28 16:25:32 2014
@@ -26,19 +26,27 @@ import javax.jcr.RepositoryException;
import javax.jcr.Session;
import com.google.common.collect.ImmutableList;
+
+import org.apache.jackrabbit.oak.spi.security.OpenSecurityProvider;
+import org.junit.After;
+import org.junit.Before;
import org.junit.Test;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
-public class NamePathTest extends AbstractRepositoryTest {
+public class NamePathTest {
+
+ private Session session;
- /**
- * logger instance
- */
- private static final Logger log =
LoggerFactory.getLogger(NamePathTest.class);
+ @Before
+ public void setup() throws RepositoryException {
+ session = new Jcr()
+ .with(new OpenSecurityProvider())
+ .createRepository()
+ .login();
+ }
- public NamePathTest(NodeStoreFixture fixture) {
- super(fixture);
+ @After
+ public void teardown() throws RepositoryException {
+ session.logout();
}
@Test
@@ -47,7 +55,7 @@ public class NamePathTest extends Abstra
"//jcr:content",
"//content"
);
- testPaths(paths, getAdminSession());
+ testPaths(paths);
}
@Test
@@ -59,7 +67,7 @@ public class NamePathTest extends Abstra
"jc/r:content",
"con/ent"
);
- testNames(names, getAdminSession());
+ testNames(names);
}
@Test
@@ -67,7 +75,7 @@ public class NamePathTest extends Abstra
List<String> paths = ImmutableList.of(
"/jcr:con:ent"
);
- testPaths(paths, getAdminSession());
+ testPaths(paths);
}
@Test
@@ -75,7 +83,7 @@ public class NamePathTest extends Abstra
List<String> names = ImmutableList.of(
"jcr:con:ent"
);
- testNames(names, getAdminSession());
+ testNames(names);
}
@Test
@@ -85,7 +93,7 @@ public class NamePathTest extends Abstra
"/jcr:con]ent",
"/con]ent"
);
- testPaths(paths, getAdminSession());
+ testPaths(paths);
}
@Test
@@ -111,7 +119,7 @@ public class NamePathTest extends Abstra
"jc[r:content",
"con[ent"
);
- testNames(names, getAdminSession());
+ testNames(names);
}
@Test
@@ -124,7 +132,7 @@ public class NamePathTest extends Abstra
"/*ontent",
"/conten*"
);
- testPaths(paths, getAdminSession());
+ testPaths(paths);
}
@Test
@@ -134,10 +142,10 @@ public class NamePathTest extends Abstra
"jcr:*ontent",
"jcr:conten*",
"con*ent",
- "*ontent", // TODO fails
+ "*ontent",
"conten*"
);
- testNames(names, getAdminSession());
+ testNames(names);
}
@Test
@@ -150,7 +158,7 @@ public class NamePathTest extends Abstra
"/conten|",
"/con|ent"
);
- testPaths(paths, getAdminSession());
+ testPaths(paths);
}
@Test
@@ -160,10 +168,10 @@ public class NamePathTest extends Abstra
"jcr:|ontent",
"jcr:conten|",
"con|ent",
- "|ontent", //TODO fails
+ "|ontent",
"conten|"
);
- testNames(names, getAdminSession());
+ testNames(names);
}
@Test
@@ -177,13 +185,13 @@ public class NamePathTest extends Abstra
"con\tent"
);
- testPaths(paths, getAdminSession());
+ testPaths(paths);
}
@Test
public void testWhitespaceInName() throws Exception {
List<String> names = ImmutableList.of(
-// "jcr:content ", // FIXME OAK-1174
+ "jcr:content ",
"content ",
" content",
"jcr:content\t",
@@ -191,24 +199,23 @@ public class NamePathTest extends Abstra
"\tcontent",
"con\tent"
);
- testNames(names, getAdminSession());
+ testNames(names);
}
@Test
public void testSpaceInNames() throws RepositoryException {
- Session session = getAdminSession();
Node n = session.getRootNode().addNode("c o n t e n t");
- Node n2 = session.getNode(n.getPath());
+ session.getNode(n.getPath());
}
- private static void testPaths(List<String> paths, Session session) throws
RepositoryException {
+ private void testPaths(List<String> paths) throws RepositoryException {
for (String path : paths) {
- testPath(path, session);
+ testPath(path);
}
}
- private static void testPath(String path, Session session) throws
RepositoryException {
+ private void testPath(String path) throws RepositoryException {
RepositoryException exception = null;
try {
session.itemExists(path);
@@ -225,13 +232,13 @@ public class NamePathTest extends Abstra
}
}
- private static void testNames(List<String> names, Session session) throws
RepositoryException {
+ private void testNames(List<String> names) throws RepositoryException {
for (String name : names) {
- testName(name, session);
+ testName(name);
}
}
- private static void testName(String name, Session session) throws
RepositoryException {
+ private void testName(String name) throws RepositoryException {
Exception exception = null;
try {
session.getRootNode().addNode(name);
