Skip to content

Commit

Permalink
Issue #12453 - AnnotationParser NPE protection
Browse files Browse the repository at this point in the history 
+ Better utilize Resource object, don't rely
  on Path existing for things that don't
  require Path.
  • Loading branch information
@joakime
joakime committed Oct 31, 2024
1 parent 94c3f9d commit f1905e6
Show file tree
Hide file tree
Showing 5 changed files with 169 additions and 70 deletions.
33 changes: 31 additions & 2 deletions jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/FileID.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;

/**
Expand All @@ -38,6 +39,7 @@ public class FileID
*/
public static String getBasename(Path path)
{
Objects.requireNonNull(path);
Path filename = path.getFileName();
if (filename == null)
return "";
Expand Down Expand Up @@ -366,11 +368,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;
Expand Down Expand Up @@ -404,6 +422,8 @@ public static boolean isClassFile(Path path)
*/
public static boolean isHidden(Path path)
{
Objects.requireNonNull(path);

int count = path.getNameCount();
for (int i = 0; i < count; i++)
{
Expand Down Expand Up @@ -443,6 +463,9 @@ public static boolean isHidden(Path path)
*/
public static boolean isHidden(Path base, Path path)
{
Objects.requireNonNull(base);
Objects.requireNonNull(path);

// Work with the path in relative form, from the base onwards to the path
return isHidden(base.relativize(path));
}
Expand Down Expand Up @@ -492,6 +515,9 @@ public static boolean isJavaArchive(String filename)
*/
public static boolean isMetaInfVersions(Path path)
{
if (path == null)
return false;

if (path.getNameCount() < 3)
return false;

Expand Down Expand Up @@ -531,6 +557,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;
Expand Down
75 changes: 54 additions & 21 deletions ...y-ee10-annotations/src/main/java/org/eclipse/jetty/ee10/annotations/AnnotationParser.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -550,21 +551,23 @@ public void parse(final Set<? extends Handler> handlers, Resource r) throws Exce
if (!r.exists())
return;

if (FileID.isJavaArchive(r.getPath()))
{
parseJar(handlers, r);
return;
}

if (r.isDirectory())
{
parseDir(handlers, r);
return;
}

if (FileID.isClassFile(r.getPath()))
else
{
parseClass(handlers, null, r.getPath());
if (FileID.isJavaArchive(r.getFileName()))
{
parseJar(handlers, r);
return;
}

if (FileID.isClassFile(r.getFileName()))
{
parseClass(handlers, null, r);
}
}

if (LOG.isDebugEnabled())
Expand Down Expand Up @@ -606,7 +609,7 @@ protected void parseDir(Set<? extends Handler> handlers, Resource dirResource) t

try
{
parseClass(handlers, dirResource, candidate.getPath());
parseClass(handlers, dirResource, candidate);
}
catch (Exception ex)
{
Expand All @@ -629,9 +632,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);

Expand All @@ -649,27 +649,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.16", 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);
}
}

Expand Down
25 changes: 14 additions & 11 deletions ...e10-osgi-boot/src/main/java/org/eclipse/jetty/ee10/osgi/annotations/AnnotationParser.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -109,25 +109,28 @@ public void parse(final Set<? extends Handler> handlers, Resource r) throws Exce
if (!r.exists())
return;

if (FileID.isJavaArchive(r.getPath()))
{
parseJar(handlers, r);
return;
}

if (r.isDirectory())
{
parseDir(handlers, r);
return;
}

if (FileID.isClassFile(r.getPath()))
else
{
parseClass(handlers, null, r.getPath());
if (FileID.isJavaArchive(r.getFileName()))
{
parseJar(handlers, r);
return;
}

if (FileID.isClassFile(r.getFileName()))
{
parseClass(handlers, null, r);
return;
}
}

//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);
Expand Down
74 changes: 55 additions & 19 deletions ...tty-ee9-annotations/src/main/java/org/eclipse/jetty/ee9/annotations/AnnotationParser.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -547,21 +548,26 @@ public void parse(final Set<? extends Handler> handlers, Resource r) throws Exce
if (r == null)
return;

if (FileID.isJavaArchive(r.getPath()))
{
parseJar(handlers, r);
if (!r.exists())
return;
}

if (r.isDirectory())
{
parseDir(handlers, r);
return;
}

if (FileID.isClassFile(r.getPath()))
else
{
parseClass(handlers, null, r.getPath());
if (FileID.isJavaArchive(r.getFileName()))
{
parseJar(handlers, r);
return;
}

if (FileID.isClassFile(r.getFileName()))
{
parseClass(handlers, null, r);
}
}

if (LOG.isDebugEnabled())
Expand Down Expand Up @@ -603,7 +609,7 @@ protected void parseDir(Set<? extends Handler> handlers, Resource dirResource) t

try
{
parseClass(handlers, dirResource, candidate.getPath());
parseClass(handlers, dirResource, candidate);
}
catch (Exception ex)
{
Expand All @@ -626,9 +632,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);

Expand All @@ -646,27 +649,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.16", 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);
}
}

Expand Down
Loading

0 comments on commit f1905e6

Please sign in to comment.