lucene-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From uschind...@apache.org
Subject svn commit: r1718069 - in /lucene/dev/trunk/lucene: CHANGES.txt expressions/src/java/org/apache/lucene/expressions/js/JavascriptCompiler.java expressions/src/test/org/apache/lucene/expressions/js/TestCustomFunctions.java tools/junit4/tests.policy
Date Sat, 05 Dec 2015 11:04:01 GMT
Author: uschindler
Date: Sat Dec  5 11:04:01 2015
New Revision: 1718069

URL: http://svn.apache.org/viewvc?rev=1718069&view=rev
Log:
LUCENE-6920: Improve custom function checks in expressions module to use MethodHandles and
work without extra security privileges

Modified:
    lucene/dev/trunk/lucene/CHANGES.txt
    lucene/dev/trunk/lucene/expressions/src/java/org/apache/lucene/expressions/js/JavascriptCompiler.java
    lucene/dev/trunk/lucene/expressions/src/test/org/apache/lucene/expressions/js/TestCustomFunctions.java
    lucene/dev/trunk/lucene/tools/junit4/tests.policy

Modified: lucene/dev/trunk/lucene/CHANGES.txt
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/lucene/CHANGES.txt?rev=1718069&r1=1718068&r2=1718069&view=diff
==============================================================================
--- lucene/dev/trunk/lucene/CHANGES.txt (original)
+++ lucene/dev/trunk/lucene/CHANGES.txt Sat Dec  5 11:04:01 2015
@@ -132,6 +132,12 @@ Bug Fixes
   (https://scan.coverity.com/projects/5620 CID 119973 and CID 120081)
   (Christine Poerschke, Coverity Scan (via Rishabh Patel))
 
+Other
+
+* LUCENE-6920: Improve custom function checks in expressions module
+  to use MethodHandles and work without extra security privileges.
+  (Uwe Schindler, Robert Muir)
+
 ======================= Lucene 5.4.0 =======================
 
 New Features

Modified: lucene/dev/trunk/lucene/expressions/src/java/org/apache/lucene/expressions/js/JavascriptCompiler.java
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/lucene/expressions/src/java/org/apache/lucene/expressions/js/JavascriptCompiler.java?rev=1718069&r1=1718068&r2=1718069&view=diff
==============================================================================
--- lucene/dev/trunk/lucene/expressions/src/java/org/apache/lucene/expressions/js/JavascriptCompiler.java
(original)
+++ lucene/dev/trunk/lucene/expressions/src/java/org/apache/lucene/expressions/js/JavascriptCompiler.java
Sat Dec  5 11:04:01 2015
@@ -18,6 +18,8 @@ package org.apache.lucene.expressions.js
 
 import java.io.IOException;
 import java.io.Reader;
+import java.lang.invoke.MethodHandles;
+import java.lang.invoke.MethodType;
 import java.lang.reflect.Constructor;
 import java.lang.reflect.Method;
 import java.lang.reflect.Modifier;
@@ -140,7 +142,8 @@ public final class JavascriptCompiler {
       throw new NullPointerException("A parent ClassLoader must be given.");
     }
     for (Method m : functions.values()) {
-      checkFunction(m, parent);
+      checkFunctionClassLoader(m, parent);
+      checkFunction(m);
     }
     return new JavascriptCompiler(sourceText, functions).compileExpression(parent);
   }
@@ -692,7 +695,7 @@ public final class JavascriptCompiler {
         @SuppressWarnings({"rawtypes", "unchecked"}) Class[] args = new Class[arity];
         Arrays.fill(args, double.class);
         Method method = clazz.getMethod(methodName, args);
-        checkFunction(method, JavascriptCompiler.class.getClassLoader());
+        checkFunction(method);
         map.put(call, method);
       }
     } catch (ReflectiveOperationException | IOException e) {
@@ -700,40 +703,44 @@ public final class JavascriptCompiler {
     }
     DEFAULT_FUNCTIONS = Collections.unmodifiableMap(map);
   }
-  
-  private static void checkFunction(Method method, ClassLoader parent) {
-    // We can only call the function if the given parent class loader of our compiled class
has access to the method:
-    final ClassLoader functionClassloader = method.getDeclaringClass().getClassLoader();
-    if (functionClassloader != null) { // it is a system class iff null!
-      boolean found = false;
-      while (parent != null) {
-        if (parent == functionClassloader) {
-          found = true;
-          break;
-        }
-        parent = parent.getParent();
-      }
-      if (!found) {
-        throw new IllegalArgumentException(method + " is not declared by a class which is
accessible by the given parent ClassLoader.");
-      }
+    
+  /** Check Method signature for compatibility. */
+  private static void checkFunction(Method method) {
+    // check that the Method is public in some public reachable class:
+    final MethodType type;
+    try {
+      type = MethodHandles.publicLookup().unreflect(method).type();
+    } catch (IllegalAccessException iae) {
+      throw new IllegalArgumentException(method + " is not accessible (declaring class or
method not public).");
     }
     // do some checks if the signature is "compatible":
     if (!Modifier.isStatic(method.getModifiers())) {
       throw new IllegalArgumentException(method + " is not static.");
     }
-    if (!Modifier.isPublic(method.getModifiers())) {
-      throw new IllegalArgumentException(method + " is not public.");
-    }
-    if (!Modifier.isPublic(method.getDeclaringClass().getModifiers())) {
-      throw new IllegalArgumentException(method.getDeclaringClass().getName() + " is not
public.");
-    }
-    for (Class<?> clazz : method.getParameterTypes()) {
-      if (!clazz.equals(double.class)) {
-        throw new IllegalArgumentException(method + " must take only double parameters");
+    for (int arg = 0, arity = type.parameterCount(); arg < arity; arg++) {
+      if (type.parameterType(arg) != double.class) {
+        throw new IllegalArgumentException(method + " must take only double parameters.");
       }
     }
-    if (method.getReturnType() != double.class) {
+    if (type.returnType() != double.class) {
       throw new IllegalArgumentException(method + " does not return a double.");
     }
   }
+  
+  /** Cross check if declaring class of given method is the same as
+   * returned by the given parent {@link ClassLoader} on string lookup.
+   * This prevents {@link NoClassDefFoundError}.
+   */
+  private static void checkFunctionClassLoader(Method method, ClassLoader parent) {
+    boolean ok = false;
+    try {
+      final Class<?> clazz = method.getDeclaringClass();
+      ok = Class.forName(clazz.getName(), false, parent) == clazz;
+    } catch (ClassNotFoundException e) {
+      ok = false;
+    }
+    if (!ok) {
+      throw new IllegalArgumentException(method + " is not declared by a class which is accessible
by the given parent ClassLoader.");
+    }
+  }
 }

Modified: lucene/dev/trunk/lucene/expressions/src/test/org/apache/lucene/expressions/js/TestCustomFunctions.java
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/lucene/expressions/src/test/org/apache/lucene/expressions/js/TestCustomFunctions.java?rev=1718069&r1=1718068&r2=1718069&view=diff
==============================================================================
--- lucene/dev/trunk/lucene/expressions/src/test/org/apache/lucene/expressions/js/TestCustomFunctions.java
(original)
+++ lucene/dev/trunk/lucene/expressions/src/test/org/apache/lucene/expressions/js/TestCustomFunctions.java
Sat Dec  5 11:04:01 2015
@@ -166,7 +166,7 @@ public class TestCustomFunctions extends
       JavascriptCompiler.compile("foo()", functions, getClass().getClassLoader());
       fail();
     } catch (IllegalArgumentException e) {
-      assertTrue(e.getMessage().contains("is not public"));
+      assertTrue(e.getMessage().contains("not public"));
     }
   }
 
@@ -182,7 +182,7 @@ public class TestCustomFunctions extends
       JavascriptCompiler.compile("foo()", functions, getClass().getClassLoader());
       fail();
     } catch (IllegalArgumentException e) {
-      assertTrue(e.getMessage().contains("is not public"));
+      assertTrue(e.getMessage().contains("not public"));
     }
   }
   

Modified: lucene/dev/trunk/lucene/tools/junit4/tests.policy
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/lucene/tools/junit4/tests.policy?rev=1718069&r1=1718068&r2=1718069&view=diff
==============================================================================
--- lucene/dev/trunk/lucene/tools/junit4/tests.policy (original)
+++ lucene/dev/trunk/lucene/tools/junit4/tests.policy Sat Dec  5 11:04:01 2015
@@ -62,8 +62,6 @@ grant {
   // analyzers/uima: needed by UIMA message localization... (?)
   permission java.lang.RuntimePermission "createSecurityManager";
   permission java.lang.RuntimePermission "createClassLoader";
-  // expressions TestCustomFunctions (only on older java8?)
-  permission java.lang.RuntimePermission "getClassLoader";
   // needed to test unmap hack on platforms that support it
   permission java.lang.RuntimePermission "accessClassInPackage.sun.misc";
   // needed by cyberneko usage by benchmarks on J9



Mime
View raw message