Improve error message when whitelist resource file is not found (#119012)

This commit replaces a NullPointerException that occurs if a whitelist
resource is not found with a customized message. Additionally it
augments the message with specific actions, especially in the case the
owning class is modularized which requies additional work.

Author: rjernst

modules/lang-painless/spi/src/main/java/org/elasticsearch/painless/spi/WhitelistLoader.java

@@ -9,8 +9,10 @@
 
 package org.elasticsearch.painless.spi;
 
+import org.elasticsearch.ResourceNotFoundException;
 import org.elasticsearch.painless.spi.annotation.WhitelistAnnotationParser;
 
+import java.io.InputStream;
 import java.io.InputStreamReader;
 import java.io.LineNumberReader;
 import java.lang.reflect.Constructor;
@@ -140,7 +142,7 @@ public static Whitelist loadFromResourceFiles(Class<?> resource, String... filep
      * }
      * }
      */
-    public static Whitelist loadFromResourceFiles(Class<?> resource, Map<String, WhitelistAnnotationParser> parsers, String... filepaths) {
+    public static Whitelist loadFromResourceFiles(Class<?> owner, Map<String, WhitelistAnnotationParser> parsers, String... filepaths) {
         List<WhitelistClass> whitelistClasses = new ArrayList<>();
         List<WhitelistMethod> whitelistStatics = new ArrayList<>();
         List<WhitelistClassBinding> whitelistClassBindings = new ArrayList<>();
@@ -153,7 +155,7 @@ public static Whitelist loadFromResourceFiles(Class<?> resource, Map<String, Whi
 
             try (
                 LineNumberReader reader = new LineNumberReader(
-                    new InputStreamReader(resource.getResourceAsStream(filepath), StandardCharsets.UTF_8)
+                    new InputStreamReader(getResourceAsStream(owner, filepath), StandardCharsets.UTF_8)
                 )
             ) {
 
@@ -483,16 +485,40 @@ public static Whitelist loadFromResourceFiles(Class<?> resource, Map<String, Whi
                 if (javaClassName != null) {
                     throw new IllegalArgumentException("invalid definition: expected closing bracket");
                 }
+            } catch (ResourceNotFoundException e) {
+                throw e; // rethrow
             } catch (Exception exception) {
                 throw new RuntimeException("error in [" + filepath + "] at line [" + number + "]", exception);
             }
         }
 
-        ClassLoader loader = AccessController.doPrivileged((PrivilegedAction<ClassLoader>) resource::getClassLoader);
+        ClassLoader loader = AccessController.doPrivileged((PrivilegedAction<ClassLoader>) owner::getClassLoader);
 
         return new Whitelist(loader, whitelistClasses, whitelistStatics, whitelistClassBindings, Collections.emptyList());
     }
 
+    private static InputStream getResourceAsStream(Class<?> owner, String name) {
+        InputStream stream = owner.getResourceAsStream(name);
+        if (stream == null) {
+            String msg = "Whitelist file ["
+                + owner.getPackageName().replace(".", "/")
+                + "/"
+                + name
+                + "] not found from owning class ["
+                + owner.getName()
+                + "].";
+            if (owner.getModule().isNamed()) {
+                msg += " Check that the file exists and the package ["
+                    + owner.getPackageName()
+                    + "] is opened "
+                    + "to module "
+                    + WhitelistLoader.class.getModule().getName();
+            }
+            throw new ResourceNotFoundException(msg);
+        }
+        return stream;
+    }
+
     private static List<Object> parseWhitelistAnnotations(Map<String, WhitelistAnnotationParser> parsers, String line) {
 
         List<Object> annotations;

modules/lang-painless/spi/src/test/java/org/elasticsearch/painless/WhitelistLoaderTests.java

@@ -9,6 +9,7 @@
 
 package org.elasticsearch.painless;
 
+import org.elasticsearch.ResourceNotFoundException;
 import org.elasticsearch.painless.spi.Whitelist;
 import org.elasticsearch.painless.spi.WhitelistClass;
 import org.elasticsearch.painless.spi.WhitelistLoader;
@@ -17,10 +18,18 @@
 import org.elasticsearch.painless.spi.annotation.NoImportAnnotation;
 import org.elasticsearch.painless.spi.annotation.WhitelistAnnotationParser;
 import org.elasticsearch.test.ESTestCase;
+import org.elasticsearch.test.compiler.InMemoryJavaCompiler;
+import org.elasticsearch.test.jar.JarUtils;
 
+import java.lang.ModuleLayer.Controller;
+import java.nio.charset.StandardCharsets;
+import java.nio.file.Path;
 import java.util.HashMap;
 import java.util.Map;
 
+import static org.hamcrest.Matchers.equalTo;
+import static org.hamcrest.Matchers.notNullValue;
+
 public class WhitelistLoaderTests extends ESTestCase {
 
     public void testUnknownAnnotations() {
@@ -96,4 +105,52 @@ public void testAnnotations() {
 
         assertEquals(3, count);
     }
+
+    public void testMissingWhitelistResource() {
+        var e = expectThrows(ResourceNotFoundException.class, () -> WhitelistLoader.loadFromResourceFiles(Whitelist.class, "missing.txt"));
+        assertThat(
+            e.getMessage(),
+            equalTo(
+                "Whitelist file [org/elasticsearch/painless/spi/missing.txt] not found"
+                    + " from owning class [org.elasticsearch.painless.spi.Whitelist]."
+            )
+        );
+    }
+
+    public void testMissingWhitelistResourceInModule() throws Exception {
+        Map<String, CharSequence> sources = new HashMap<>();
+        sources.put("module-info", "module m {}");
+        sources.put("p.TestOwner", "package p; public class TestOwner { }");
+        var classToBytes = InMemoryJavaCompiler.compile(sources);
+
+        Path dir = createTempDir(getTestName());
+        Path jar = dir.resolve("m.jar");
+        Map<String, byte[]> jarEntries = new HashMap<>();
+        jarEntries.put("module-info.class", classToBytes.get("module-info"));
+        jarEntries.put("p/TestOwner.class", classToBytes.get("p.TestOwner"));
+        jarEntries.put("p/resource.txt", "# test resource".getBytes(StandardCharsets.UTF_8));
+        JarUtils.createJarWithEntries(jar, jarEntries);
+
+        try (var loader = JarUtils.loadJar(jar)) {
+            Controller controller = JarUtils.loadModule(jar, loader.classloader(), "m");
+            Module module = controller.layer().findModule("m").orElseThrow();
+
+            Class<?> ownerClass = module.getClassLoader().loadClass("p.TestOwner");
+
+            // first check we get a nice error message when accessing the resource
+            var e = expectThrows(ResourceNotFoundException.class, () -> WhitelistLoader.loadFromResourceFiles(ownerClass, "resource.txt"));
+            assertThat(
+                e.getMessage(),
+                equalTo(
+                    "Whitelist file [p/resource.txt] not found from owning class [p.TestOwner]."
+                        + " Check that the file exists and the package [p] is opened to module null"
+                )
+            );
+
+            // now check we can actually read it once the package is opened to us
+            controller.addOpens(module, "p", WhitelistLoader.class.getModule());
+            var whitelist = WhitelistLoader.loadFromResourceFiles(ownerClass, "resource.txt");
+            assertThat(whitelist, notNullValue());
+        }
+    }
 }

server/src/main/resources/org/elasticsearch/bootstrap/test-framework.policy

@@ -88,6 +88,7 @@ grant codeBase "${codebase.elasticsearch}" {
 // this is the test-framework, but the jar is horribly named
 grant codeBase "${codebase.framework}" {
   permission java.lang.RuntimePermission "setSecurityManager";
+  permission java.lang.RuntimePermission "createClassLoader";
 };
 
 grant codeBase "${codebase.elasticsearch-rest-client}" {
@@ -129,4 +130,5 @@ grant {
   permission java.nio.file.LinkPermission "symbolic";
   // needed for keystore tests
   permission java.lang.RuntimePermission "accessUserInformation";
+  permission java.lang.RuntimePermission "getClassLoader";
 };

test/framework/src/main/java/org/elasticsearch/test/jar/JarUtils.java

@@ -9,13 +9,24 @@
 
 package org.elasticsearch.test.jar;
 
+import org.elasticsearch.test.PrivilegedOperations.ClosableURLClassLoader;
+
 import java.io.ByteArrayInputStream;
 import java.io.IOException;
 import java.io.OutputStream;
+import java.lang.module.Configuration;
+import java.lang.module.ModuleFinder;
+import java.net.MalformedURLException;
+import java.net.URL;
+import java.net.URLClassLoader;
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.nio.file.StandardOpenOption;
+import java.security.AccessController;
+import java.security.PrivilegedAction;
+import java.util.List;
 import java.util.Map;
+import java.util.Set;
 import java.util.jar.JarEntry;
 import java.util.jar.JarOutputStream;
 import java.util.jar.Manifest;
@@ -85,6 +96,28 @@ public static void createJarWithEntriesUTF(Path jarfile, Map<String, String> ent
         createJarWithEntries(jarfile, map);
     }
 
+    /**
+     * Creates a class loader for the given jar file.
+     * @param path Path to the jar file to load
+     * @return A URLClassLoader that will load classes from the jar. It should be closed when no longer needed.
+     */
+    public static ClosableURLClassLoader loadJar(Path path) {
+        try {
+            URL[] urls = new URL[] { path.toUri().toURL() };
+            return new ClosableURLClassLoader(URLClassLoader.newInstance(urls, JarUtils.class.getClassLoader()));
+        } catch (MalformedURLException e) {
+            throw new RuntimeException(e);
+        }
+    }
+
+    public static ModuleLayer.Controller loadModule(Path path, ClassLoader loader, String name) {
+        var finder = ModuleFinder.of(path.getParent());
+        var cf = Configuration.resolveAndBind(finder, List.of(ModuleLayer.boot().configuration()), ModuleFinder.of(), Set.of(name));
+        return AccessController.doPrivileged(
+            (PrivilegedAction<ModuleLayer.Controller>) () -> ModuleLayer.defineModulesWithOneLoader(cf, List.of(ModuleLayer.boot()), loader)
+        );
+    }
+
     @FunctionalInterface
     interface UncheckedIOFunction<T, R> {
         R apply(T t) throws IOException;