bval-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From mben...@apache.org
Subject [3/4] bval git commit: refactor DefaultMessageInterpolator to harden message interpolation
Date Mon, 15 Oct 2018 23:26:24 GMT
refactor DefaultMessageInterpolator to harden message interpolation


Project: http://git-wip-us.apache.org/repos/asf/bval/repo
Commit: http://git-wip-us.apache.org/repos/asf/bval/commit/c2a80702
Tree: http://git-wip-us.apache.org/repos/asf/bval/tree/c2a80702
Diff: http://git-wip-us.apache.org/repos/asf/bval/diff/c2a80702

Branch: refs/heads/bv2
Commit: c2a80702a1d99045029f888d537776ae4eb65c0c
Parents: 1398ecc
Author: Matt Benson <mbenson@apache.org>
Authored: Wed Oct 10 20:59:00 2018 -0500
Committer: Matt Benson <mbenson@apache.org>
Committed: Mon Oct 15 18:25:58 2018 -0500

----------------------------------------------------------------------
 bval-jsr/pom.xml                                |  24 +++
 .../main/java/org/apache/bval/el/ELFacade.java  |  54 +++---
 .../bval/jsr/DefaultMessageInterpolator.java    | 178 +++++++++---------
 .../bval/jsr/util/LookBehindRegexHolder.java    | 117 ++++++++++++
 .../jsr/DefaultMessageInterpolatorTest.java     | 186 ++++++++++++++++++-
 .../jsr/util/LookBehindRegexHolderTest.java     |  80 ++++++++
 .../resources/ValidationMessages.properties     |   2 +
 7 files changed, 535 insertions(+), 106 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/bval/blob/c2a80702/bval-jsr/pom.xml
----------------------------------------------------------------------
diff --git a/bval-jsr/pom.xml b/bval-jsr/pom.xml
index d810e0d..09573f9 100644
--- a/bval-jsr/pom.xml
+++ b/bval-jsr/pom.xml
@@ -211,6 +211,30 @@
             <artifactId>mockito-core</artifactId>
             <scope>test</scope>
         </dependency>
+        <dependency>
+            <groupId>org.apache.tomcat</groupId>
+            <artifactId>tomcat-jasper-el</artifactId>
+            <version>7.0.91</version>
+            <scope>test</scope>
+        </dependency>
+        <dependency>
+            <groupId>org.glassfish</groupId>
+            <artifactId>javax.el</artifactId>
+            <version>3.0.1-b10</version>
+            <scope>test</scope>
+        </dependency>
+        <dependency>
+            <groupId>de.odysseus.juel</groupId>
+            <artifactId>juel-api</artifactId>
+            <version>2.2.7</version>
+            <scope>test</scope>
+        </dependency>
+        <dependency>
+            <groupId>de.odysseus.juel</groupId>
+            <artifactId>juel-impl</artifactId>
+            <version>2.2.7</version>
+            <scope>test</scope>
+        </dependency>
     </dependencies>
 
     <build>

http://git-wip-us.apache.org/repos/asf/bval/blob/c2a80702/bval-jsr/src/main/java/org/apache/bval/el/ELFacade.java
----------------------------------------------------------------------
diff --git a/bval-jsr/src/main/java/org/apache/bval/el/ELFacade.java b/bval-jsr/src/main/java/org/apache/bval/el/ELFacade.java
index 9798455..3248f8a 100644
--- a/bval-jsr/src/main/java/org/apache/bval/el/ELFacade.java
+++ b/bval-jsr/src/main/java/org/apache/bval/el/ELFacade.java
@@ -16,6 +16,11 @@
  */
 package org.apache.bval.el;
 
+import java.lang.reflect.Method;
+import java.util.Formatter;
+import java.util.HashMap;
+import java.util.Map;
+
 import javax.el.ArrayELResolver;
 import javax.el.BeanELResolver;
 import javax.el.CompositeELResolver;
@@ -28,41 +33,48 @@ import javax.el.MapELResolver;
 import javax.el.ResourceBundleELResolver;
 import javax.el.ValueExpression;
 import javax.el.VariableMapper;
-import java.lang.reflect.Method;
-import java.util.Formatter;
-import java.util.HashMap;
-import java.util.Map;
+
+import org.apache.bval.jsr.util.LookBehindRegexHolder;
 
 // ELProcessor or JavaEE 7 would be perfect too but this impl can be used in javaee 6
 public final class ELFacade implements MessageEvaluator {
-    private static final ExpressionFactory EXPRESSION_FACTORY;
-    static {
-        ExpressionFactory ef;
-        try {
-            ef = ExpressionFactory.newInstance();
-        } catch (final Exception e) {
-            ef = null;
+    private enum EvaluationType {
+        IMMEDIATE("\\$"), DEFERRED("#");
+
+        /**
+         * {@link LookBehindRegexHolder} to recognize a non-escaped EL
+         * expression of this evaluation type, hallmarked by a trigger
+         * character.
+         */
+        private final LookBehindRegexHolder regex;
+
+        private EvaluationType(String trigger) {
+            this.regex = new LookBehindRegexHolder(
+                String.format("(?<!(?:^|[^\\\\])(?:\\\\\\\\){0,%%d}\\\\)%s\\{", trigger),
n -> (n - 3) / 2);
         }
-        EXPRESSION_FACTORY = ef;
     }
+
     private static final ELResolver RESOLVER = initResolver();
 
+    private final ExpressionFactory expressionFactory = ExpressionFactory.newInstance();
+
     @Override
     public String interpolate(final String message, final Map<String, Object> annotationParameters,
         final Object validatedValue) {
         try {
-            if (EXPRESSION_FACTORY != null) {
+            if (EvaluationType.IMMEDIATE.regex.matcher(message).find()) {
                 final BValELContext context = new BValELContext();
                 final VariableMapper variables = context.getVariableMapper();
                 annotationParameters.forEach(
-                    (k, v) -> variables.setVariable(k, EXPRESSION_FACTORY.createValueExpression(v,
Object.class)));
+                    (k, v) -> variables.setVariable(k, expressionFactory.createValueExpression(v,
Object.class)));
 
                 variables.setVariable("validatedValue",
-                    EXPRESSION_FACTORY.createValueExpression(validatedValue, Object.class));
+                    expressionFactory.createValueExpression(validatedValue, Object.class));
 
-                // #{xxx} shouldn't be evaluated
-                return EXPRESSION_FACTORY.createValueExpression(context, message.replace("#{",
"\\#{"), String.class)
-                    .getValue(context).toString();
+                // Java Bean Validation does not support EL expressions that look like JSP
"deferred" expressions
+                return expressionFactory.createValueExpression(context,
+                    EvaluationType.DEFERRED.regex.matcher(message).replaceAll("\\$0"), String.class).getValue(context)
+                    .toString();
             }
         } catch (final Exception e) {
             // no-op
@@ -81,7 +93,7 @@ public final class ELFacade implements MessageEvaluator {
         return resolver;
     }
 
-    private static class BValELContext extends ELContext {
+    private class BValELContext extends ELContext {
         private final FunctionMapper functions = new BValFunctionMapper();
         private final VariableMapper variables = new BValVariableMapper();
 
@@ -108,13 +120,13 @@ public final class ELFacade implements MessageEvaluator {
         }
     }
 
-    private static class BValVariableMapper extends VariableMapper {
+    private class BValVariableMapper extends VariableMapper {
         private final Map<String, ValueExpression> variables = new HashMap<String,
ValueExpression>();
 
         @Override
         public ValueExpression resolveVariable(final String variable) {
             if ("formatter".equals(variable)) {
-                return EXPRESSION_FACTORY.createValueExpression(new BValFormatter(), Object.class);
+                return expressionFactory.createValueExpression(new BValFormatter(), Object.class);
             }
             return variables.get(variable);
         }

http://git-wip-us.apache.org/repos/asf/bval/blob/c2a80702/bval-jsr/src/main/java/org/apache/bval/jsr/DefaultMessageInterpolator.java
----------------------------------------------------------------------
diff --git a/bval-jsr/src/main/java/org/apache/bval/jsr/DefaultMessageInterpolator.java b/bval-jsr/src/main/java/org/apache/bval/jsr/DefaultMessageInterpolator.java
index 624a32b..b0697b3 100644
--- a/bval-jsr/src/main/java/org/apache/bval/jsr/DefaultMessageInterpolator.java
+++ b/bval-jsr/src/main/java/org/apache/bval/jsr/DefaultMessageInterpolator.java
@@ -16,23 +16,25 @@
  */
 package org.apache.bval.jsr;
 
-import org.apache.bval.el.MessageEvaluator;
-import org.apache.bval.util.reflection.Reflection;
-import org.apache.commons.weaver.privilizer.Privilizing;
-import org.apache.commons.weaver.privilizer.Privilizing.CallTo;
-
-import javax.validation.MessageInterpolator;
-
+import java.lang.reflect.InvocationTargetException;
 import java.util.Arrays;
 import java.util.Locale;
 import java.util.Map;
 import java.util.MissingResourceException;
+import java.util.Optional;
 import java.util.ResourceBundle;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.logging.Level;
 import java.util.logging.Logger;
 import java.util.regex.Matcher;
-import java.util.regex.Pattern;
+
+import javax.validation.MessageInterpolator;
+
+import org.apache.bval.el.MessageEvaluator;
+import org.apache.bval.jsr.util.LookBehindRegexHolder;
+import org.apache.bval.util.reflection.Reflection;
+import org.apache.commons.weaver.privilizer.Privilizing;
+import org.apache.commons.weaver.privilizer.Privilizing.CallTo;
 
 /**
  * Description: Resource bundle backed message interpolator.
@@ -47,8 +49,11 @@ public class DefaultMessageInterpolator implements MessageInterpolator
{
     private static final String DEFAULT_VALIDATION_MESSAGES = "org.apache.bval.jsr.ValidationMessages";
     private static final String USER_VALIDATION_MESSAGES = "ValidationMessages";
 
-    /** Regular expression used to do message interpolation. */
-    private static final Pattern messageParameterPattern = Pattern.compile("(\\{[\\w\\.]+\\})");
+    /**
+     * {@link LookBehindRegexHolder} to match Bean Validation attribute patterns, considering
character escaping.
+     */
+    private static final LookBehindRegexHolder MESSAGE_PARAMETER = new LookBehindRegexHolder(
+        "(?<!(?:^|[^\\\\])(?:\\\\\\\\){0,%1$d}\\\\)\\{((?:[\\w\\.]|\\\\[\\{\\$\\}\\\\])+)\\}",
n -> (n - 4) / 2);
 
     /** The default locale for the current user. */
     private Locale defaultLocale;
@@ -153,11 +158,32 @@ public class DefaultMessageInterpolator implements MessageInterpolator
{
             resolvedMessage = evaluator.interpolate(resolvedMessage, annotationParameters,
validatedValue);
         }
 
-        // curly braces need to be scaped in the original msg, so unescape them now
-        resolvedMessage =
-            resolvedMessage.replace("\\{", "{").replace("\\}", "}").replace("\\\\", "\\").replace("\\$",
"$");
+        return resolveEscapeSequences(resolvedMessage);
+    }
+
+    private String resolveEscapeSequences(String s) {
+        int pos = s.indexOf('\\');
+        if (pos < 0) {
+            return s;
+        }
+        StringBuilder result = new StringBuilder(s.length());
+ 
+        int prev = 0;
+        do {
+            if (pos + 1 >= s.length()) {
+                break;
+            }
+            if ("\\{}$".indexOf(s.charAt(pos + 1)) >= 0) {
+                result.append(s, prev, pos);
+                prev = pos + 1;
+            }
+            pos = s.indexOf('\\', pos + 2);
+        } while (pos > 0);
 
-        return resolvedMessage;
+        if (prev < s.length()) {
+            result.append(s, prev, s.length());
+        }
+        return result.toString();
     }
 
     private boolean hasReplacementTakenPlace(String origMessage, String newMessage) {
@@ -173,13 +199,13 @@ public class DefaultMessageInterpolator implements MessageInterpolator
{
     private ResourceBundle getFileBasedResourceBundle(Locale locale) {
         ResourceBundle rb;
         final ClassLoader classLoader = Reflection.getClassLoader(DefaultMessageInterpolator.class);
-        if (classLoader != null) {
-            rb = loadBundle(classLoader, locale, USER_VALIDATION_MESSAGES + " not found by
thread local classloader");
-        } else {
-        // 2011-03-27 jw: No privileged action required.
-        // A class can always access the classloader of itself and of subclasses.
+        if (classLoader == null) {
+            // 2011-03-27 jw: No privileged action required.
+            // A class can always access the classloader of itself and of subclasses.
             rb = loadBundle(getClass().getClassLoader(), locale,
-                USER_VALIDATION_MESSAGES + " not found by validator classloader");
+            USER_VALIDATION_MESSAGES + " not found by validator classloader");
+        } else {
+            rb = loadBundle(classLoader, locale, USER_VALIDATION_MESSAGES + " not found by
thread local classloader");
         }
         if (LOG_FINEST) {
             if (rb == null) {
@@ -197,85 +223,81 @@ public class DefaultMessageInterpolator implements MessageInterpolator
{
             return ResourceBundle.getBundle(USER_VALIDATION_MESSAGES, locale, classLoader);
         } catch (final MissingResourceException e) {
             log.fine(message);
+            return null;
         }
-        return null;
     }
 
     private String replaceVariables(String message, ResourceBundle bundle, Locale locale,
boolean recurse) {
-        final Matcher matcher = messageParameterPattern.matcher(message);
-        final StringBuffer sb = new StringBuffer(64);
+        final Matcher matcher = MESSAGE_PARAMETER.matcher(message);
+        final StringBuilder sb = new StringBuilder(64);
+        int prev = 0;
         while (matcher.find()) {
-            final String parameter = matcher.group(1);
-            String resolvedParameterValue = resolveParameter(parameter, bundle, locale, recurse);
-            matcher.appendReplacement(sb, sanitizeForAppendReplacement(resolvedParameterValue));
+            int start = matcher.start();
+            if (start > prev) {
+                sb.append(message, prev, start);
+            }
+            sb.append(resolveParameter(matcher.group(1), bundle, locale, recurse).orElseGet(matcher::group));
+            prev = matcher.end();
+        }
+        if (prev < message.length()) {
+            sb.append(message, prev, message.length());
         }
-        matcher.appendTail(sb);
         return sb.toString();
     }
 
     private String replaceAnnotationAttributes(final String message, final Map<String,
Object> annotationParameters) {
-        Matcher matcher = messageParameterPattern.matcher(message);
-        StringBuffer sb = new StringBuffer(64);
+        final Matcher matcher = MESSAGE_PARAMETER.matcher(message);
+        final StringBuilder sb = new StringBuilder(64);
+        int prev = 0;
         while (matcher.find()) {
+            int start = matcher.start();
             String resolvedParameterValue;
             String parameter = matcher.group(1);
-            Object variable = annotationParameters.get(removeCurlyBrace(parameter));
-            if (variable != null) {
-                if (variable.getClass().isArray()) {
-                    resolvedParameterValue = Arrays.toString((Object[]) variable);
-                } else {
-                    resolvedParameterValue = variable.toString();
+            Object variable = annotationParameters.get(parameter);
+            if (variable == null) {
+                resolvedParameterValue = matcher.group();
+            } else if (Object[].class.isInstance(variable)) {
+                resolvedParameterValue = Arrays.toString((Object[]) variable);
+            } else if (variable.getClass().isArray()) {
+                try {
+                    resolvedParameterValue = (String) Reflection
+                        .getDeclaredMethod(Arrays.class, "toString", variable.getClass()).invoke(null,
variable);
+                } catch (IllegalAccessException | IllegalArgumentException | InvocationTargetException
e) {
+                    throw new IllegalStateException("Could not expand array " + variable);
                 }
             } else {
-                resolvedParameterValue = parameter;
+                resolvedParameterValue = variable.toString();
             }
-            matcher.appendReplacement(sb, sanitizeForAppendReplacement(resolvedParameterValue));
+            if (start > prev) {
+                sb.append(message, prev, start);
+            }
+            sb.append(resolvedParameterValue);
+            prev = matcher.end();
+        }
+        if (prev < message.length()) {
+            sb.append(message, prev, message.length());
         }
-        matcher.appendTail(sb);
         return sb.toString();
     }
 
-    private String resolveParameter(String parameterName, ResourceBundle bundle, Locale locale,
boolean recurse) {
-        String parameterValue;
-        try {
-            if (bundle == null) {
-                parameterValue = parameterName;
-            } else {
-                parameterValue = bundle.getString(removeCurlyBrace(parameterName));
-                if (recurse) {
-                    parameterValue = replaceVariables(parameterValue, bundle, locale, recurse);
-                }
+    private Optional<String> resolveParameter(String parameterName, ResourceBundle
bundle, Locale locale,
+        boolean recurse) {
+        return Optional.ofNullable(bundle).map(b -> {
+            try {
+                return b.getString(parameterName);
+            } catch (final MissingResourceException e) {
+                return null;
             }
-        } catch (final MissingResourceException e) {
-            // return parameter itself
-            parameterValue = parameterName;
-        }
-
-        return parameterValue;
-    }
-
-    private String removeCurlyBrace(String parameter) {
-        return parameter.substring(1, parameter.length() - 1);
+        }).map(v -> recurse ? replaceVariables(v, bundle, locale, recurse) : v);
     }
 
     private ResourceBundle findDefaultResourceBundle(Locale locale) {
-        ResourceBundle bundle = defaultBundlesMap.get(locale);
-        if (bundle == null) {
-            bundle = ResourceBundle.getBundle(DEFAULT_VALIDATION_MESSAGES, locale);
-            defaultBundlesMap.put(locale, bundle);
-        }
-        return bundle;
+        return defaultBundlesMap.computeIfAbsent(locale,
+            k -> ResourceBundle.getBundle(DEFAULT_VALIDATION_MESSAGES, locale));
     }
 
     private ResourceBundle findUserResourceBundle(Locale locale) {
-        ResourceBundle bundle = userBundlesMap.get(locale);
-        if (bundle == null) {
-            bundle = getFileBasedResourceBundle(locale);
-            if (bundle != null) {
-                userBundlesMap.put(locale, bundle);
-            }
-        }
-        return bundle;
+        return userBundlesMap.computeIfAbsent(locale, this::getFileBasedResourceBundle);
     }
 
     /**
@@ -285,16 +307,4 @@ public class DefaultMessageInterpolator implements MessageInterpolator
{
     public void setLocale(Locale locale) {
         defaultLocale = locale;
     }
-
-    /**
-     * Escapes the string to comply with
-     * {@link Matcher#appendReplacement(StringBuffer, String)} requirements.
-     *
-     * @param src
-     *            The original string.
-     * @return The sanitized string.
-     */
-    private String sanitizeForAppendReplacement(String src) {
-        return src.replace("\\", "\\\\").replace("$", "\\$");
-    }
 }

http://git-wip-us.apache.org/repos/asf/bval/blob/c2a80702/bval-jsr/src/main/java/org/apache/bval/jsr/util/LookBehindRegexHolder.java
----------------------------------------------------------------------
diff --git a/bval-jsr/src/main/java/org/apache/bval/jsr/util/LookBehindRegexHolder.java b/bval-jsr/src/main/java/org/apache/bval/jsr/util/LookBehindRegexHolder.java
new file mode 100644
index 0000000..6040b1c
--- /dev/null
+++ b/bval-jsr/src/main/java/org/apache/bval/jsr/util/LookBehindRegexHolder.java
@@ -0,0 +1,117 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with this
+ * work for additional information regarding copyright ownership. The ASF
+ * licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations under
+ * the License.
+ */
+package org.apache.bval.jsr.util;
+
+import java.util.function.IntUnaryOperator;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+
+import org.apache.bval.util.Validate;
+
+/**
+ * Utility class to manage regular expressions that require simulated infinite
+ * lookbehind, e.g. to determine whether a sequence of escape characters is
+ * complete unto itself.
+ */
+public class LookBehindRegexHolder {
+    public static final int DEFAULT_INITIAL_MAXIMUM_LENGTH = 256;
+    public static final int DEFAULT_EXPANSION_BLOCK_SIZE = 128;
+
+    private final String regex;
+    private final int expansionBlockSize;
+    private final IntUnaryOperator computeInjectedRepetition;
+
+    private volatile int maximumLength;
+    private Pattern pattern;
+
+    /**
+     * Create a new {@link LookBehindRegexHolder} instance with the default
+     * initial maximum length and expansion block size.
+     * 
+     * @param regex
+     *            assumed to contain a {@code %d} Java format sequence
+     * @param computeInjectedRepetition
+     *            function to compute the injected number of repetitions to
+     *            inject at {@code %d} in {@code regex}
+     */
+    public LookBehindRegexHolder(String regex, IntUnaryOperator computeInjectedRepetition)
{
+        this(regex, DEFAULT_INITIAL_MAXIMUM_LENGTH, DEFAULT_EXPANSION_BLOCK_SIZE, computeInjectedRepetition);
+    }
+
+    /**
+     * Create a new {@link LookBehindRegexHolder} instance.
+     * 
+     * @param regex
+     *            assumed to contain a {@code %d} Java format sequence
+     * @param initialMaximumLength
+     *            initial guess
+     * @param expansionBlockSize
+     *            number of bytes by which to increase the maximum length when a
+     *            {@link Matcher} is requested for a larger message size
+     * @param computeInjectedRepetition
+     *            function to compute the injected number of repetitions to
+     *            inject at {@code %d} in {@code regex}
+     */
+    public LookBehindRegexHolder(String regex, int initialMaximumLength, int expansionBlockSize,
+        IntUnaryOperator computeInjectedRepetition) {
+        super();
+        Validate.isTrue(regex != null && !regex.trim().isEmpty(), "empty regex");
+        Validate.isTrue(initialMaximumLength > 0, "invalid initial maximum length %d",
initialMaximumLength);
+        Validate.isTrue(expansionBlockSize > 0, "Invalid expansion block size %d", expansionBlockSize);
+        Validate.notNull(computeInjectedRepetition, "missing %s to compute injected repetition",
+            IntUnaryOperator.class.getSimpleName());
+        this.regex = regex;
+        this.expansionBlockSize = expansionBlockSize;
+        this.computeInjectedRepetition = computeInjectedRepetition;
+        accommodate(initialMaximumLength);
+    }
+
+    /**
+     * Get a {@link Matcher} against the specified {@link CharSequence}.
+     * 
+     * @param s
+     * @return {@link Matcher}
+     */
+    public Matcher matcher(CharSequence s) {
+        if (s.length() > maximumLength) {
+            accommodate(s.length());
+        }
+        return pattern.matcher(s);
+    }
+
+    int getMaximumLength() {
+        return this.maximumLength;
+    }
+
+    String getPattern() {
+        return pattern.pattern();
+    }
+
+    private synchronized void accommodate(int maximumLength) {
+        if (this.maximumLength < maximumLength) {
+            if (this.maximumLength == 0) {
+                this.maximumLength = maximumLength;
+            } else {
+                int difference = maximumLength - this.maximumLength;
+                int addBlocks = difference / expansionBlockSize + 1;
+                this.maximumLength += addBlocks * expansionBlockSize;
+            }
+            this.pattern =
+                Pattern.compile(String.format(regex, computeInjectedRepetition.applyAsInt(this.maximumLength)));
+        }
+    }
+}

http://git-wip-us.apache.org/repos/asf/bval/blob/c2a80702/bval-jsr/src/test/java/org/apache/bval/jsr/DefaultMessageInterpolatorTest.java
----------------------------------------------------------------------
diff --git a/bval-jsr/src/test/java/org/apache/bval/jsr/DefaultMessageInterpolatorTest.java
b/bval-jsr/src/test/java/org/apache/bval/jsr/DefaultMessageInterpolatorTest.java
index 6bd42c5..a7d37e1 100644
--- a/bval-jsr/src/test/java/org/apache/bval/jsr/DefaultMessageInterpolatorTest.java
+++ b/bval-jsr/src/test/java/org/apache/bval/jsr/DefaultMessageInterpolatorTest.java
@@ -16,14 +16,25 @@
  */
 package org.apache.bval.jsr;
 
+import static org.hamcrest.CoreMatchers.anyOf;
+import static org.hamcrest.CoreMatchers.equalTo;
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assume.assumeThat;
+import static org.junit.Assume.assumeTrue;
 
 import java.lang.annotation.Annotation;
+import java.net.URL;
+import java.net.URLClassLoader;
+import java.util.Arrays;
+import java.util.List;
 import java.util.Locale;
 import java.util.Objects;
 import java.util.function.Predicate;
 import java.util.function.Supplier;
 
+import javax.el.ExpressionFactory;
 import javax.validation.MessageInterpolator;
 import javax.validation.Validator;
 import javax.validation.constraints.Digits;
@@ -33,13 +44,33 @@ import javax.validation.metadata.ConstraintDescriptor;
 import org.apache.bval.constraints.NotEmpty;
 import org.apache.bval.jsr.example.Author;
 import org.apache.bval.jsr.example.PreferredGuest;
+import org.junit.After;
+import org.junit.AfterClass;
 import org.junit.Before;
 import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+import org.junit.runners.Parameterized.Parameters;
 
 /**
  * MessageResolverImpl Tester.
  */
+@RunWith(Parameterized.class)
 public class DefaultMessageInterpolatorTest {
+    @Parameters(name="{0}")
+    public static List<Object[]> generateParameters(){
+        return Arrays.asList(new Object[] { "default", null },
+            new Object[] { "ri", "com.sun.el.ExpressionFactoryImpl" },
+            new Object[] { "tomcat", "org.apache.el.ExpressionFactoryImpl" },
+            new Object[] { "juel", "de.odysseus.el.ExpressionFactoryImpl" },
+            new Object[] { "invalid", "java.lang.Object" });
+    }
+
+    @AfterClass
+    public static void cleanup() {
+        System.clearProperty(ExpressionFactory.class.getName());
+    }
+
     private static Predicate<ConstraintDescriptor<?>> forConstraintType(Class<?
extends Annotation> type) {
         return d -> Objects.equals(type, d.getAnnotation().annotationType());
     }
@@ -64,16 +95,49 @@ public class DefaultMessageInterpolatorTest {
         };
     }
 
+    private String elImpl;
+    private String elFactory;
     private DefaultMessageInterpolator interpolator;
     private Validator validator;
+    private boolean elAvailable;
+    private ClassLoader originalClassLoader;
+
+    public DefaultMessageInterpolatorTest(String elImpl, String elFactory) {
+        this.elImpl = elImpl;
+        this.elFactory = elFactory;
+    }
 
     @Before
     public void setUp() throws Exception {
+        // store and replace CCL to sidestep EL factory caching
+        originalClassLoader = Thread.currentThread().getContextClassLoader();
+        Thread.currentThread().setContextClassLoader(new URLClassLoader(new URL[] {}, originalClassLoader));
+        
+        try {
+            Class<?> elFactoryClass;
+            if (elFactory == null) {
+                elFactoryClass = ExpressionFactory.class;
+                System.clearProperty(ExpressionFactory.class.getName());
+            } else {
+                elFactoryClass = Class.forName(elFactory);
+                System.setProperty(ExpressionFactory.class.getName(), elFactory);
+            }
+            assertTrue(elFactoryClass.isInstance(ExpressionFactory.newInstance()));
+            elAvailable = true;
+        } catch (Exception e) {
+            elAvailable = false;
+        }
         interpolator = new DefaultMessageInterpolator();
         interpolator.setLocale(Locale.ENGLISH);
         validator = ApacheValidatorFactory.getDefault().getValidator();
     }
 
+    @After
+    public void tearDownEL() {
+        assumeTrue(originalClassLoader != null);
+        Thread.currentThread().setContextClassLoader(originalClassLoader);
+    }
+
     @Test
     public void testInterpolateFromValidationResources() {
         String msg = interpolator.interpolate("{validator.creditcard}",
@@ -124,6 +188,127 @@ public class DefaultMessageInterpolatorTest {
             otherIdResult);
     }
 
+    @Test
+    public void testRecursiveInterpolation() {
+        String msg = this.interpolator.interpolate("{recursive.interpolation.1}",
+            context("12345678",
+                () -> validator.getConstraintsForClass(Person.class).getConstraintsForProperty("idNumber")
+                    .getConstraintDescriptors().stream().filter(forConstraintType(Pattern.class)).findFirst()
+                    .orElseThrow(() -> new AssertionError("expected constraint missing"))));
+
+        assertEquals("must match \"....$\"", msg);
+    }
+
+    @Test
+    public void testNoELAvailable() {
+        assumeThat(elImpl, equalTo("invalid"));
+        assertFalse(elAvailable);
+
+        assertEquals("${regexp.charAt(4)}", interpolator.interpolate("${regexp.charAt(4)}",
+            context("12345678",
+                () -> validator.getConstraintsForClass(Person.class).getConstraintsForProperty("idNumber")
+                .getConstraintDescriptors().stream().filter(forConstraintType(Pattern.class)).findFirst()
+                .orElseThrow(() -> new AssertionError("expected constraint missing")))));
+    }
+
+    @Test
+    public void testExpressionLanguageEvaluation() {
+        assumeTrue(elAvailable);
+        
+        assertEquals("Expected value of length 8 to match pattern",
+            interpolator.interpolate("Expected value of length ${validatedValue.length()}
to match pattern",
+                context("12345678",
+                    () -> validator.getConstraintsForClass(Person.class).getConstraintsForProperty("idNumber")
+                    .getConstraintDescriptors().stream().filter(forConstraintType(Pattern.class)).findFirst()
+                    .orElseThrow(() -> new AssertionError("expected constraint missing")))));
+    }
+    
+    @Test
+    public void testMixedEvaluation() {
+        assumeTrue(elAvailable);
+
+        assertEquals("Expected value of length 8 to match pattern ....$",
+            interpolator.interpolate("Expected value of length ${validatedValue.length()}
to match pattern {regexp}",
+                context("12345678",
+                    () -> validator.getConstraintsForClass(Person.class).getConstraintsForProperty("idNumber")
+                        .getConstraintDescriptors().stream().filter(forConstraintType(Pattern.class)).findFirst()
+                        .orElseThrow(() -> new AssertionError("expected constraint missing")))));
+    }
+
+    @Test
+    public void testELEscapingTomcatJuel() {
+        assumeTrue(elAvailable);
+        assumeThat(elImpl, anyOf(equalTo("tomcat"), equalTo("juel")));
+
+        // not so much a test as an illustration that the specified EL implementations are
seemingly confused by leading
+        // backslashes and treats the whole expression as literal. We could skip any literal
text before the first
+        // non-escaped $, but that would only expose us to inconsistency for composite expressions
containing more
+        // than one component EL expression
+
+        assertEquals("${regexp.charAt(4)}", interpolator.interpolate("\\${regexp.charAt(4)}",
+            context("12345678",
+                () -> validator.getConstraintsForClass(Person.class).getConstraintsForProperty("idNumber")
+                .getConstraintDescriptors().stream().filter(forConstraintType(Pattern.class)).findFirst()
+                .orElseThrow(() -> new AssertionError("expected constraint missing")))));
+
+        assertEquals("${regexp.charAt(4)}", interpolator.interpolate("\\\\${regexp.charAt(4)}",
+            context("12345678",
+                () -> validator.getConstraintsForClass(Person.class).getConstraintsForProperty("idNumber")
+                .getConstraintDescriptors().stream().filter(forConstraintType(Pattern.class)).findFirst()
+                .orElseThrow(() -> new AssertionError("expected constraint missing")))));
+    }
+
+    @Test
+    public void testELEscapingRI() {
+        assumeTrue(elAvailable);
+        assumeThat(elImpl, equalTo("ri"));
+
+        assertEquals("returns literal", "${regexp.charAt(4)}",
+            interpolator.interpolate("\\${regexp.charAt(4)}",
+                context("12345678",
+                    () -> validator.getConstraintsForClass(Person.class).getConstraintsForProperty("idNumber")
+                        .getConstraintDescriptors().stream().filter(forConstraintType(Pattern.class)).findFirst()
+                        .orElseThrow(() -> new AssertionError("expected constraint missing")))));
+
+        assertEquals("returns literal \\ followed by $, later interpreted as an escape sequence",
"$",
+            interpolator.interpolate("\\\\${regexp.charAt(4)}",
+                context("12345678",
+                    () -> validator.getConstraintsForClass(Person.class).getConstraintsForProperty("idNumber")
+                        .getConstraintDescriptors().stream().filter(forConstraintType(Pattern.class)).findFirst()
+                        .orElseThrow(() -> new AssertionError("expected constraint missing")))));
+
+        assertEquals("returns literal \\ followed by .", "\\.",
+            interpolator.interpolate("\\\\${regexp.charAt(3)}",
+                context("12345678",
+                    () -> validator.getConstraintsForClass(Person.class).getConstraintsForProperty("idNumber")
+                        .getConstraintDescriptors().stream().filter(forConstraintType(Pattern.class)).findFirst()
+                        .orElseThrow(() -> new AssertionError("expected constraint missing")))));
+    }
+
+    @Test
+    public void testEscapedELPattern() {
+        assertEquals("$must match \"....$\"",
+            interpolator.interpolate("\\${javax.validation.constraints.Pattern.message}",
+                context("12345678",
+                    () -> validator.getConstraintsForClass(Person.class).getConstraintsForProperty("idNumber")
+                        .getConstraintDescriptors().stream().filter(forConstraintType(Pattern.class)).findFirst()
+                        .orElseThrow(() -> new AssertionError("expected constraint missing")))));
+
+        assertEquals("$must match \"....$\"",
+            interpolator.interpolate("\\${javax.validation.constraints.Pattern.message}",
+                context("12345678",
+                    () -> validator.getConstraintsForClass(Person.class).getConstraintsForProperty("idNumber")
+                    .getConstraintDescriptors().stream().filter(forConstraintType(Pattern.class)).findFirst()
+                    .orElseThrow(() -> new AssertionError("expected constraint missing")))));
+
+        assertEquals("\\$must match \"....$\"",
+            interpolator.interpolate("\\\\\\${javax.validation.constraints.Pattern.message}",
+                context("12345678",
+                    () -> validator.getConstraintsForClass(Person.class).getConstraintsForProperty("idNumber")
+                    .getConstraintDescriptors().stream().filter(forConstraintType(Pattern.class)).findFirst()
+                    .orElseThrow(() -> new AssertionError("expected constraint missing")))));
+    }
+
     public static class Person {
 
         @Pattern(message = "Id number should match {regexp}", regexp = "....$")
@@ -131,6 +316,5 @@ public class DefaultMessageInterpolatorTest {
 
         @Pattern(message = "Other id should match {regexp}", regexp = ".\\n")
         public String otherId;
-
     }
 }

http://git-wip-us.apache.org/repos/asf/bval/blob/c2a80702/bval-jsr/src/test/java/org/apache/bval/jsr/util/LookBehindRegexHolderTest.java
----------------------------------------------------------------------
diff --git a/bval-jsr/src/test/java/org/apache/bval/jsr/util/LookBehindRegexHolderTest.java
b/bval-jsr/src/test/java/org/apache/bval/jsr/util/LookBehindRegexHolderTest.java
new file mode 100644
index 0000000..c36a800
--- /dev/null
+++ b/bval-jsr/src/test/java/org/apache/bval/jsr/util/LookBehindRegexHolderTest.java
@@ -0,0 +1,80 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with this
+ * work for additional information regarding copyright ownership. The ASF
+ * licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations under
+ * the License.
+ */
+package org.apache.bval.jsr.util;
+
+import static org.junit.Assert.*;
+
+import org.junit.Before;
+import org.junit.Test;
+
+public class LookBehindRegexHolderTest {
+    private static final String MESSAGE_PARAMETER_PATTERN =
+        "(?<!(?:^|[^\\\\])(?:\\\\\\\\){0,%1$d}\\\\)\\{((?:[\\w\\.]|\\\\[\\{\\$\\}\\\\])+)\\}";
+
+    private LookBehindRegexHolder messageParameter;
+
+    @Before
+    public void setup() {
+        messageParameter = new LookBehindRegexHolder(MESSAGE_PARAMETER_PATTERN, 5, 5, this::computeInjectedRepetition);
+    }
+
+    @Test
+    public void testLookBehind() {
+        assertFound("{foo}");
+        assertFound("${foo}");
+        assertNotFound("\\{foo}");
+        assertNotFound("{foo\\}");
+        assertFound("\\\\{foo}");
+        assertFound("{foo\\\\}");
+        assertNotFound("\\\\\\{foo}");
+        assertNotFound("{foo\\\\\\}");
+        assertFound("\\${foo}");
+        assertFound("\\\\${foo}");
+        assertFound("\\\\\\${foo}");
+        assertFound("\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\${foo}");
+        assertFound("{foo\\\\\\\\\\\\\\\\}");
+    }
+
+    private void assertFound(String msg) {
+        assertTrue(messageParameter.matcher(msg).find());
+    }
+
+    private void assertNotFound(String msg) {
+        assertFalse(messageParameter.matcher(msg).find());
+    }
+
+    @Test
+    public void testGrowth() {
+        assertEquals(5, messageParameter.getMaximumLength());
+        assertMessageSizeYieldsMaximumSize(5, 5);
+        assertMessageSizeYieldsMaximumSize(10, 6);
+        assertMessageSizeYieldsMaximumSize(10, 5);
+        assertMessageSizeYieldsMaximumSize(10, 9);
+        assertMessageSizeYieldsMaximumSize(35, 31);
+    }
+
+    private void assertMessageSizeYieldsMaximumSize(int max, int msg) {
+        messageParameter.matcher(new String(new byte[msg]));
+        assertEquals(max, messageParameter.getMaximumLength());
+        assertEquals(String.format(MESSAGE_PARAMETER_PATTERN, computeInjectedRepetition(max)),
+            messageParameter.getPattern());
+    }
+
+    private int computeInjectedRepetition(int maximumLength) {
+        return (maximumLength - 5) / 2;
+    }
+}

http://git-wip-us.apache.org/repos/asf/bval/blob/c2a80702/bval-jsr/src/test/resources/ValidationMessages.properties
----------------------------------------------------------------------
diff --git a/bval-jsr/src/test/resources/ValidationMessages.properties b/bval-jsr/src/test/resources/ValidationMessages.properties
index aad269b..c7f2bda 100644
--- a/bval-jsr/src/test/resources/ValidationMessages.properties
+++ b/bval-jsr/src/test/resources/ValidationMessages.properties
@@ -24,3 +24,5 @@ test.validator.creditcard=credit card is not valid
 # custom messages (examples) for validation-api-1.0.CR1
 validator.creditcard=credit card is not valid
 
+recursive.interpolation.1={recursive.interpolation.2}
+recursive.interpolation.2={javax.validation.constraints.Pattern.message}


Mime
View raw message