From 2a20225bd7102d0ecf5e1efa5c0a85f20e5418db Mon Sep 17 00:00:00 2001 From: Stephan Schroevers Date: Sun, 28 Dec 2014 15:35:19 +0100 Subject: [PATCH] Fix various issues with JarFileScanner. Overview: - Previously JarFileScanner assumed backslashes to be the platform independent file separator for zip files. This is incorrect (and in fact, the associated unit tests did assume forward slashes, using "javax/ws/rs" to denote the package "javax.ws.rs"). See [1] for details. - As a result the non-recursive scanning mode was broken: failing to find the backslash delimiter, the scanner would return classes from subpackages even when it was instructed not to do so. This has been fixed. - The scanner also did not verify that "matching" classes were in a proper subdirectory of the indicated package. As a result a class such as javax.ws.rs.GET was matched for the following "packages": - javax/ws/r (too short, misses an "s") - javax/ws/rs/GE (too long, includes class name prefix) This has been fixed as well. The class' unit tests were extended to cover the problematic cases. Of the five new unit tests, three fail prior to the fix. [1] https://pkware.cachefly.net/webdocs/casestudies/APPNOTE.TXT --- .../internal/scanning/JarFileScanner.java | 17 ++-- .../internal/scanning/JarFileScannerTest.java | 89 +++++++++++++------ 2 files changed, 72 insertions(+), 34 deletions(-) diff --git a/core-server/src/main/java/org/glassfish/jersey/server/internal/scanning/JarFileScanner.java b/core-server/src/main/java/org/glassfish/jersey/server/internal/scanning/JarFileScanner.java index f6fc72ea15..bea871bfa8 100644 --- a/core-server/src/main/java/org/glassfish/jersey/server/internal/scanning/JarFileScanner.java +++ b/core-server/src/main/java/org/glassfish/jersey/server/internal/scanning/JarFileScanner.java @@ -58,7 +58,7 @@ public final class JarFileScanner implements ResourceFinder { private static final Logger LOGGER = Logger.getLogger(JarFileScanner.class.getName()); // platform independent file separator within the jar file - private static final char JAR_FILE_SEPARATOR = '\\'; + private static final char JAR_FILE_SEPARATOR = '/'; private final JarInputStream jarInputStream; private final String parent; @@ -91,14 +91,17 @@ public boolean hasNext() { break; } if (!next.isDirectory() && next.getName().startsWith(parent)) { + final String suffix = next.getName().substring(parent.length()); if (recursive) { // accept any entries with the prefix - break; - } - // accept only entries directly in the folder. - final String suffix = next.getName().substring(parent.length()); - if (suffix.lastIndexOf(JAR_FILE_SEPARATOR) <= 0) { - break; + if (parent.isEmpty() || suffix.indexOf(JAR_FILE_SEPARATOR) == 0) { + break; + } + } else { + // accept only entries directly in the folder. + if (suffix.lastIndexOf(JAR_FILE_SEPARATOR) == (parent.isEmpty() ? -1 : 0)) { + break; + } } } } while (true); diff --git a/core-server/src/test/java/org/glassfish/jersey/server/internal/scanning/JarFileScannerTest.java b/core-server/src/test/java/org/glassfish/jersey/server/internal/scanning/JarFileScannerTest.java index 5a5ea8d301..9af3ea6e10 100644 --- a/core-server/src/test/java/org/glassfish/jersey/server/internal/scanning/JarFileScannerTest.java +++ b/core-server/src/test/java/org/glassfish/jersey/server/internal/scanning/JarFileScannerTest.java @@ -40,21 +40,33 @@ package org.glassfish.jersey.server.internal.scanning; import java.io.FileInputStream; +import java.io.IOException; import java.io.InputStream; import java.util.Enumeration; import java.util.jar.JarEntry; import java.util.jar.JarFile; +import java.util.regex.Pattern; import org.junit.Before; import org.junit.Test; +import org.junit.experimental.theories.DataPoint; +import org.junit.experimental.theories.Theories; +import org.junit.experimental.theories.Theory; +import org.junit.runner.RunWith; import static org.hamcrest.CoreMatchers.equalTo; +import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertThat; import static org.junit.Assert.fail; /** * @author Martin Snyder */ +@RunWith(Theories.class) public class JarFileScannerTest { + @DataPoint + public static final boolean RECURSIVE = true; + @DataPoint + public static final boolean NON_RECURSIVE = false; private String jaxRsApiPath; @@ -75,55 +87,78 @@ public void setUp() throws Exception { } } - /** - * Test case for JERSEY-2197, JERSEY-2175. - */ @Test - public void testClassEnumeration() throws Exception { - // Count actual entries. - final JarFile jarFile = new JarFile(jaxRsApiPath); + public void testRecursiveResourceEnumerationOfAllPackages() throws IOException { + final int actualEntries = countJarEntriesByPattern(Pattern.compile(".*\\.(class|properties|xml)")); + final int scannedEntries = countJarEntriesUsingScanner("", true); + assertThat("Failed to enumerate all contents of javax.ws.rs-api", scannedEntries, equalTo(actualEntries)); + } - int actualEntries = 0; - try { - final Enumeration entries = jarFile.entries(); + @Test + public void testRecursiveClassEnumerationWithExistantPackage() throws IOException { + final int actualEntries = countJarEntriesByPattern(Pattern.compile("javax/ws/rs/.*\\.class")); + final int scannedEntries = countJarEntriesUsingScanner("javax/ws/rs", true); + assertThat("Failed to enumerate all contents of javax.ws.rs-api", scannedEntries, equalTo(actualEntries)); + } + @Test + public void testNonRecursiveClassEnumerationWithExistantPackage() throws IOException { + final int actualEntries = countJarEntriesByPattern(Pattern.compile("javax/ws/rs/[^/]*\\.class")); + final int scannedEntries = countJarEntriesUsingScanner("javax/ws/rs", false); + assertThat("Failed to enumerate package 'javax.ws.rs' of javax.ws.rs-api", scannedEntries, equalTo(actualEntries)); + } + + private int countJarEntriesByPattern(final Pattern pattern) throws IOException { + int matchingEntries = 0; + + try (final JarFile jarFile = new JarFile(this.jaxRsApiPath)) { + final Enumeration entries = jarFile.entries(); while (entries.hasMoreElements()) { final JarEntry entry = entries.nextElement(); - - if (entry.getName().endsWith(".class")) { - actualEntries++; + if (pattern.matcher(entry.getName()).matches()) { + matchingEntries++; } } - } finally { - jarFile.close(); } - // Scan entries using Jersey scanner. - final InputStream jaxRsApi = new FileInputStream(jaxRsApiPath); + return matchingEntries; + } - try { - final JarFileScanner jarFileScanner = new JarFileScanner(jaxRsApi, "javax/ws/rs", true); + private int countJarEntriesUsingScanner(final String parent, final boolean recursive) throws IOException { + int scannedEntryCount = 0; - int scannedEntryCount = 0; + try (final InputStream jaxRsApi = new FileInputStream(this.jaxRsApiPath)) { + final JarFileScanner jarFileScanner = new JarFileScanner(jaxRsApi, parent, recursive); while (jarFileScanner.hasNext()) { // Fetch next entry. jarFileScanner.next(); + // JERSEY-2175 and JERSEY-2197: // This test doesn't actually do anything with the input stream, but it is important that it // open/close the stream to simulate actual usage. The reported defect is only exposed if you // call open/close in some fashion. - final InputStream classStream = jarFileScanner.open(); - - try { + try (final InputStream classStream = jarFileScanner.open()) { scannedEntryCount++; - } finally { - classStream.close(); } } + } + + return scannedEntryCount; + } + + @Theory + public void testClassEnumerationWithNonexistentPackage(final boolean recursive) throws IOException { + try (final InputStream jaxRsApi = new FileInputStream(this.jaxRsApiPath)) { + final JarFileScanner jarFileScanner = new JarFileScanner(jaxRsApi, "javax/ws/r", recursive); + assertFalse("Unexpectedly found package 'javax.ws.r' in javax.ws.rs-api", jarFileScanner.hasNext()); + } + } - assertThat("Failed to enumerate all contents of javax.ws.rs-api.", scannedEntryCount, equalTo(actualEntries)); - } finally { - jaxRsApi.close(); + @Theory + public void testClassEnumerationWithClassPrefix(final boolean recursive) throws IOException { + try (final InputStream jaxRsApi = new FileInputStream(this.jaxRsApiPath)) { + final JarFileScanner jarFileScanner = new JarFileScanner(jaxRsApi, "javax/ws/rs/GE", recursive); + assertFalse("Unexpectedly found package 'javax.ws.rs.GE' in javax.ws.rs-api", jarFileScanner.hasNext()); } } } \ No newline at end of file