Issue #12453 - AnnotationParser NPE protection (#13162)

+ Better utilize Resource object, don't rely on Path existing for things that don't require Path.

Author: joakime

jetty-core/jetty-annotations/src/main/java/org/eclipse/jetty/annotations/AnnotationParser.java

@@ -25,6 +25,7 @@
 import java.util.Set;
 import java.util.concurrent.ConcurrentHashMap;
 
+import org.eclipse.jetty.io.IOResources;
 import org.eclipse.jetty.util.ExceptionUtil;
 import org.eclipse.jetty.util.FileID;
 import org.eclipse.jetty.util.StringUtil;
@@ -606,7 +607,7 @@ protected void parseDir(Set<? extends Handler> handlers, Resource dirResource) t
 
             try
             {
-                parseClass(handlers, dirResource, candidate.getPath());
+                parseClass(handlers, dirResource, candidate);
             }
             catch (Exception ex)
             {
@@ -629,9 +630,6 @@ protected void parseJar(Set<? extends Handler> handlers, Resource jarResource) t
         if (jarResource == null)
             return;
 
-        /*        if (!FileID.isJavaArchive(jarResource.getPath()))
-            return;*/
-
         if (LOG.isDebugEnabled())
             LOG.debug("Scanning jar {}", jarResource);
 
@@ -649,27 +647,60 @@ protected void parseJar(Set<? extends Handler> handlers, Resource jarResource) t
      * @param containingResource the dir or jar that the class is contained within, can be null if not known
      * @param classFile the class file to parse
      * @throws IOException if unable to parse
+     * @deprecated use {@link #parseClass(Set, Resource, Resource)} instead (which uses {@link Resource} instead of {@link Path})
      */
+    @Deprecated(since = "12.0.21", forRemoval = true)
     protected void parseClass(Set<? extends Handler> handlers, Resource containingResource, Path classFile) throws IOException
     {
-        if (LOG.isDebugEnabled())
-            LOG.debug("Parse class from {}", classFile.toUri());
+        try (InputStream inputStream = Files.newInputStream(classFile))
+        {
+            parseClass(handlers, containingResource, classFile.toUri(), inputStream);
+        }
+    }
+
+    /**
+     * Use ASM on a class
+     *
+     * @param handlers the handlers to look for classes in
+     * @param containingResource the dir or jar that the class is contained within, can be null if not known
+     * @param classFile the class file to parse
+     * @throws IOException if unable to parse
+     */
+    protected void parseClass(Set<? extends Handler> handlers, Resource containingResource, Resource classFile) throws IOException
+    {
+        try (InputStream inputStream = IOResources.asInputStream(classFile))
+        {
+            parseClass(handlers, containingResource, classFile.getURI(), inputStream);
+        }
+    }
 
-        URI location = classFile.toUri();
+    /**
+     * Use ASM on a class
+     *
+     * @param handlers the handlers to look for classes in
+     * @param containingResource the dir or jar that the class is contained within, can be null if not known
+     * @param classFileRef the URI reference to the classfile location
+     * @param inputStream the class file contents to parse
+     * @throws IOException if unable to parse
+     */
+    private void parseClass(Set<? extends Handler> handlers, Resource containingResource, URI classFileRef, InputStream inputStream) throws IOException
+    {
+        if (LOG.isDebugEnabled())
+            LOG.debug("Parse class from {}", classFileRef);
 
-        try (InputStream in = Files.newInputStream(classFile))
+        try
         {
-            ClassReader reader = new ClassReader(in);
+            ClassReader reader = new ClassReader(inputStream);
             reader.accept(new MyClassVisitor(handlers, containingResource, _asmVersion), ClassReader.SKIP_CODE | ClassReader.SKIP_DEBUG | ClassReader.SKIP_FRAMES);
 
             String classname = normalize(reader.getClassName());
-            URI existing = _parsedClassNames.putIfAbsent(classname, location);
+            URI existing = _parsedClassNames.putIfAbsent(classname, classFileRef);
             if (existing != null)
-                LOG.warn("{} scanned from multiple locations: {}, {}", classname, existing, location);
+                LOG.warn("{} scanned from multiple locations: {}, {}", classname, existing, classFileRef);
         }
         catch (IllegalArgumentException | IOException e)
         {
-            throw new IOException("Unable to parse class: " + classFile.toUri(), e);
+            throw new IOException("Unable to parse class: " + classFileRef, e);
         }
     }
 

jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/FileID.java

@@ -19,6 +19,7 @@
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.util.Locale;
+import java.util.Objects;
 import java.util.regex.Pattern;
 
 /**
@@ -38,6 +39,7 @@ public class FileID
      */
     public static String getBasename(Path path)
     {
+        Objects.requireNonNull(path);
         Path filename = path.getFileName();
         if (filename == null)
             return "";
@@ -378,11 +380,27 @@ public static boolean isLibArchive(URI uri)
      */
     public static boolean isClassFile(Path path)
     {
+        if (path == null)
+            return false;
+
         Path fileNamePath = path.getFileName();
         if (fileNamePath == null)
             return false;
-        
-        String filename = fileNamePath.toString();
+
+        return isClassFile(fileNamePath.toString());
+    }
+
+    /**
+     * Predicate to test for class files
+     *
+     * @param filename the filename to test
+     * @return true if the filename ends with {@code .class}
+     */
+    public static boolean isClassFile(String filename)
+    {
+        if (StringUtil.isBlank(filename))
+            return false;
+
         // has to end in ".class"
         if (!StringUtil.asciiEndsWithIgnoreCase(filename, ".class"))
             return false;
@@ -416,6 +434,9 @@ public static boolean isClassFile(Path path)
      */
     public static boolean isHidden(Path path)
     {
+        if (path == null)
+            return false;
+
         int count = path.getNameCount();
         for (int i = 0; i < count; i++)
         {
@@ -455,6 +476,9 @@ public static boolean isHidden(Path path)
      */
     public static boolean isHidden(Path base, Path path)
     {
+        if (base == null || path == null)
+            return false;
+
         // Work with the path in relative form, from the base onwards to the path
         return isHidden(base.relativize(path));
     }
@@ -504,6 +528,9 @@ public static boolean isJavaArchive(String filename)
      */
     public static boolean isMetaInfVersions(Path path)
     {
+        if (path == null)
+            return false;
+
         if (path.getNameCount() < 3)
             return false;
 
@@ -543,6 +570,9 @@ public static boolean isNotMetaInfVersions(Path path)
      */
     public static boolean isModuleInfoClass(Path path)
     {
+        if (path == null)
+            return false;
+
         Path filenameSegment = path.getFileName();
         if (filenameSegment == null)
             return false;

jetty-ee9/jetty-ee9-osgi/jetty-ee9-osgi-boot/src/main/java/org/eclipse/jetty/ee9/osgi/annotations/AnnotationParser.java

@@ -20,6 +20,7 @@
 import org.eclipse.jetty.osgi.BundleIndex;
 import org.eclipse.jetty.util.FileID;
 import org.eclipse.jetty.util.resource.Resource;
+import org.eclipse.jetty.util.resource.Resources;
 import org.osgi.framework.Bundle;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -66,6 +67,9 @@ public void parse(final Set<? extends Handler> handlers, Resource r) throws Exce
         if (r == null)
             return;
 
+        if (!Resources.exists(r))
+            return;
+
         if (FileID.isJavaArchive(r.getPath()))
         {
             parseJar(handlers, r);
@@ -80,11 +84,11 @@ public void parse(final Set<? extends Handler> handlers, Resource r) throws Exce
 
         if (FileID.isClassFile(r.getPath()))
         {
-            parseClass(handlers, null, r.getPath());
+            parseClass(handlers, null, r);
         }
 
-        //Not already parsed, it could be a file that actually is compressed but does not have
-        //.jar/.zip etc extension, such as equinox urls, so try to parse it
+        // Not already parsed, it could be a file that actually is compressed but does not have
+        // .jar/.zip etc extension, such as equinox urls, so try to parse it
         try
         {
             parseJar(handlers, r);