From 48ca3307775c6868345291e0bb4514a3ae7c1cee Mon Sep 17 00:00:00 2001 From: Jukka Zitting Date: Wed, 27 Mar 2013 15:10:25 +0200 Subject: [PATCH] OAK-108: Shortcut path conversions Skip validation of paths in NamePathMapperImpl. Instead validate paths in ValueFactoryImpl where the TCK (indirectly) expects names and paths to be validated. Also the name tests in query code need updating. --- .../oak/namepath/NamePathMapperImpl.java | 11 ++---- .../oak/plugins/value/ValueFactoryImpl.java | 10 +++-- .../jackrabbit/oak/query/ast/NodeNameImpl.java | 45 +++++++++------------- 3 files changed, 28 insertions(+), 38 deletions(-) diff --git a/oak-core/src/main/java/org/apache/jackrabbit/oak/namepath/NamePathMapperImpl.java b/oak-core/src/main/java/org/apache/jackrabbit/oak/namepath/NamePathMapperImpl.java index 4b4313f..094196b 100644 --- a/oak-core/src/main/java/org/apache/jackrabbit/oak/namepath/NamePathMapperImpl.java +++ b/oak-core/src/main/java/org/apache/jackrabbit/oak/namepath/NamePathMapperImpl.java @@ -189,16 +189,11 @@ public class NamePathMapperImpl implements NamePathMapper { // try a shortcut if (!hasNameStartingWithDot && !hasClarkBrackets && !hasIndexBrackets) { if (!hasColon || !hasSessionLocalMappings()) { - if (JcrPathParser.validate(jcrPath)) { - if(hasTrailingSlash){ - return jcrPath.substring(0, length - 1); - } + if (hasTrailingSlash){ + return jcrPath.substring(0, length - 1); + } else { return jcrPath; } - else { - log.debug("Invalid path: {}", jcrPath); - return null; - } } } diff --git a/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/value/ValueFactoryImpl.java b/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/value/ValueFactoryImpl.java index d0a817f..1d0ae17 100644 --- a/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/value/ValueFactoryImpl.java +++ b/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/value/ValueFactoryImpl.java @@ -36,6 +36,8 @@ import org.apache.jackrabbit.oak.api.Blob; import org.apache.jackrabbit.oak.api.BlobFactory; import org.apache.jackrabbit.oak.api.PropertyState; import org.apache.jackrabbit.oak.api.PropertyValue; +import org.apache.jackrabbit.oak.namepath.JcrNameParser; +import org.apache.jackrabbit.oak.namepath.JcrPathParser; import org.apache.jackrabbit.oak.namepath.NamePathMapper; import org.apache.jackrabbit.oak.core.IdentifierManager; import org.apache.jackrabbit.oak.plugins.memory.BinaryPropertyState; @@ -214,7 +216,7 @@ public class ValueFactoryImpl implements ValueFactory { return createValue(Conversions.convert(value).toBoolean()); case PropertyType.NAME: String oakName = namePathMapper.getOakNameOrNull(value); - if (oakName == null) { + if (oakName == null || !JcrNameParser.validate(oakName)) { throw new ValueFormatException("Invalid name: " + value); } return new ValueImpl(GenericPropertyState.nameProperty("", oakName), namePathMapper); @@ -224,9 +226,9 @@ public class ValueFactoryImpl implements ValueFactory { // identifier path; do no change } else { oakValue = namePathMapper.getOakPath(value); - } - if (oakValue == null) { - throw new ValueFormatException("Invalid path: " + value); + if (oakValue == null || !JcrPathParser.validate(oakValue)) { + throw new ValueFormatException("Invalid path: " + value); + } } return new ValueImpl(GenericPropertyState.pathProperty("", oakValue), namePathMapper); case PropertyType.REFERENCE: diff --git a/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/NodeNameImpl.java b/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/NodeNameImpl.java index a21bb81..9a3e7b8 100644 --- a/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/NodeNameImpl.java +++ b/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/NodeNameImpl.java @@ -23,6 +23,7 @@ import javax.jcr.PropertyType; import org.apache.jackrabbit.oak.api.PropertyValue; import org.apache.jackrabbit.oak.api.Type; import org.apache.jackrabbit.oak.commons.PathUtils; +import org.apache.jackrabbit.oak.namepath.JcrNameParser; import org.apache.jackrabbit.oak.query.index.FilterImpl; import org.apache.jackrabbit.oak.spi.query.PropertyValues; import org.apache.jackrabbit.util.ISO9075; @@ -71,18 +72,10 @@ public class NodeNameImpl extends DynamicOperandImpl { if (v == null) { return; } - if (!isName(v)) { + String name = getName(v); + if (name == null) { throw new IllegalArgumentException("Invalid name value: " + v.toString()); } - String path = v.getValue(Type.STRING); - // Name escaping (convert _x0020_ to space) - path = decodeName(path); - if (PathUtils.isAbsolute(path)) { - throw new IllegalArgumentException("NAME() comparison with absolute path are not allowed: " + path); - } - if (PathUtils.getDepth(path) > 1) { - throw new IllegalArgumentException("NAME() comparison with relative path are not allowed: " + path); - } // TODO support NAME(..) index conditions } @@ -91,21 +84,14 @@ public class NodeNameImpl extends DynamicOperandImpl { return s == selector; } - private String decodeName(String path) { - // Name escaping (convert _x0020_ to space) - path = ISO9075.decode(path); - // normalize paths (./name > name) - path = PropertyValues.getOakPath(path, query.getNamePathMapper()); - return path; - } - /** - * Validate that the given value can be converted to a JCR name. + * Validate that the given value can be converted to a JCR name, and + * return the name. * * @param v the value - * @return true if it can be converted + * @return name value, or {@code null} if the value can not be converted */ - private static boolean isName(PropertyValue v) { + private String getName(PropertyValue v) { // TODO correctly validate JCR names - see JCR 2.0 spec 3.2.4 Naming Restrictions switch (v.getType().tag()) { case PropertyType.DATE: @@ -113,13 +99,20 @@ public class NodeNameImpl extends DynamicOperandImpl { case PropertyType.DOUBLE: case PropertyType.LONG: case PropertyType.BOOLEAN: - return false; + return null; } - String n = v.getValue(Type.STRING); - if (n.startsWith("[") && !n.endsWith("]")) { - return false; + String name = v.getValue(Type.NAME); + // Name escaping (convert _x0020_ to space) + name = ISO9075.decode(name); + // normalize paths (./name > name) + name = PropertyValues.getOakPath(name, query.getNamePathMapper()); + + if (name.startsWith("[") && !name.endsWith("]")) { + return null; + } else if (!JcrNameParser.validate(name)) { + return null; } - return true; + return name; } @Override -- 1.8.0.msysgit.0