From aad1b6870b5ee89d87dced5af21be650f66fdc44 Mon Sep 17 00:00:00 2001 From: Jan Mosig Date: Thu, 28 Jan 2021 18:08:50 +0100 Subject: [PATCH] [MSHADE-366] - "Access denied" during 'minimizeJar' Now ignoring directories when scanning the classpath for services. --- .../plugins/shade/filter/MinijarFilter.java | 87 ++++++++++--------- .../shade/filter/MinijarFilterTest.java | 75 ++++++++++------ 2 files changed, 97 insertions(+), 65 deletions(-) diff --git a/src/main/java/org/apache/maven/plugins/shade/filter/MinijarFilter.java b/src/main/java/org/apache/maven/plugins/shade/filter/MinijarFilter.java index 818339ed..48f8f189 100644 --- a/src/main/java/org/apache/maven/plugins/shade/filter/MinijarFilter.java +++ b/src/main/java/org/apache/maven/plugins/shade/filter/MinijarFilter.java @@ -35,6 +35,8 @@ import java.io.IOException; import java.io.InputStream; import java.io.InputStreamReader; +import java.nio.file.Files; +import java.nio.file.Paths; import java.util.Collections; import java.util.Enumeration; import java.util.HashSet; @@ -131,56 +133,63 @@ private void removeServices( final MavenProject project, final Clazzpath cp ) { for ( final String fileName : project.getRuntimeClasspathElements() ) { - try ( final JarFile jar = new JarFile( fileName ) ) + if ( !Files.isDirectory( Paths.get( fileName ) ) ) { - for ( final Enumeration entries = jar.entries(); entries.hasMoreElements(); ) + try ( final JarFile jar = new JarFile( fileName ) ) { - final JarEntry jarEntry = entries.nextElement(); - if ( jarEntry.isDirectory() || !jarEntry.getName().startsWith( "META-INF/services/" ) ) + for ( final Enumeration entries = jar.entries(); entries.hasMoreElements(); ) { - continue; - } - - final String serviceClassName = - jarEntry.getName().substring( "META-INF/services/".length() ); - final boolean isNeededClass = neededClasses.contains( cp.getClazz( serviceClassName ) ); - if ( !isNeededClass ) - { - continue; - } - - try ( final BufferedReader bufferedReader = - new BufferedReader( new InputStreamReader( jar.getInputStream( jarEntry ), UTF_8 ) ) ) - { - for ( String line = bufferedReader.readLine(); line != null; - line = bufferedReader.readLine() ) + final JarEntry jarEntry = entries.nextElement(); + if ( jarEntry.isDirectory() || !jarEntry.getName().startsWith( "META-INF/services/" ) ) { - final String className = line.split( "#", 2 )[0].trim(); - if ( className.isEmpty() ) - { - continue; - } - - final Clazz clazz = cp.getClazz( className ); - if ( clazz == null || !removable.contains( clazz ) ) + continue; + } + + final String serviceClassName = + jarEntry.getName().substring( "META-INF/services/".length() ); + final boolean isNeededClass = neededClasses.contains( cp.getClazz( serviceClassName ) ); + if ( !isNeededClass ) + { + continue; + } + + try ( final BufferedReader bufferedReader = + new BufferedReader( new InputStreamReader( jar.getInputStream( jarEntry ), UTF_8 ) ) ) + { + for ( String line = bufferedReader.readLine(); line != null; + line = bufferedReader.readLine() ) { - continue; + final String className = line.split( "#", 2 )[0].trim(); + if ( className.isEmpty() ) + { + continue; + } + + final Clazz clazz = cp.getClazz( className ); + if ( clazz == null || !removable.contains( clazz ) ) + { + continue; + } + + log.debug( className + " was not removed because it is a service" ); + removeClass( clazz ); + repeatScan = true; // check whether the found classes use services in turn } - - log.debug( className + " was not removed because it is a service" ); - removeClass( clazz ); - repeatScan = true; // check whether the found classes use services in turn + } + catch ( final IOException e ) + { + log.warn( e.getMessage() ); } } - catch ( final IOException e ) - { - log.warn( e.getMessage() ); - } + } + catch ( final IOException e ) + { + log.warn( e.getMessage() ); } } - catch ( final IOException e ) + else { - log.warn( e.getMessage() ); + log.debug( "Not a JAR file candidate. Ignoring classpath element '" + fileName + "'." ); } } } diff --git a/src/test/java/org/apache/maven/plugins/shade/filter/MinijarFilterTest.java b/src/test/java/org/apache/maven/plugins/shade/filter/MinijarFilterTest.java index bfbaee24..58e14d26 100644 --- a/src/test/java/org/apache/maven/plugins/shade/filter/MinijarFilterTest.java +++ b/src/test/java/org/apache/maven/plugins/shade/filter/MinijarFilterTest.java @@ -20,39 +20,48 @@ */ import static org.junit.Assert.assertEquals; +import static org.junit.Assert.fail; import static org.junit.Assume.assumeFalse; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; -import java.io.File; -import java.io.IOException; -import java.util.Set; -import java.util.TreeSet; - import org.apache.maven.artifact.Artifact; import org.apache.maven.artifact.DefaultArtifact; +import org.apache.maven.artifact.DependencyResolutionRequiredException; import org.apache.maven.plugin.logging.Log; import org.apache.maven.project.MavenProject; import org.junit.Before; +import org.junit.Rule; import org.junit.Test; import org.junit.rules.TemporaryFolder; import org.mockito.ArgumentCaptor; +import java.io.File; +import java.io.IOException; +import java.util.Arrays; +import java.util.Set; +import java.util.TreeSet; + public class MinijarFilterTest { + @Rule + public TemporaryFolder tempFolder = TemporaryFolder.builder().assureDeletion().build(); + private File emptyFile; + private Log log; + private ArgumentCaptor logCaptor; @Before public void init() throws IOException { - TemporaryFolder tempFolder = new TemporaryFolder(); - tempFolder.create(); this.emptyFile = tempFolder.newFile(); - + this.log = mock(Log.class); + logCaptor = ArgumentCaptor.forClass(CharSequence.class); } /** @@ -64,12 +73,8 @@ public void testWithMockProject() { assumeFalse( "Expected to run under JDK8+", System.getProperty("java.version").startsWith("1.7") ); - ArgumentCaptor logCaptor = ArgumentCaptor.forClass( CharSequence.class ); - MavenProject mavenProject = mockProject( emptyFile ); - Log log = mock( Log.class ); - MinijarFilter mf = new MinijarFilter( mavenProject, log ); mf.finished(); @@ -84,14 +89,10 @@ public void testWithMockProject() public void testWithPomProject() throws IOException { - ArgumentCaptor logCaptor = ArgumentCaptor.forClass( CharSequence.class ); - // project with pom packaging and no artifact. MavenProject mavenProject = mockProject( null ); mavenProject.setPackaging( "pom" ); - Log log = mock( Log.class ); - MinijarFilter mf = new MinijarFilter( mavenProject, log ); mf.finished(); @@ -105,7 +106,7 @@ public void testWithPomProject() } - private MavenProject mockProject( File file ) + private MavenProject mockProject( File file, String... classPathElements ) { MavenProject mavenProject = mock( MavenProject.class ); @@ -129,17 +130,18 @@ private MavenProject mockProject( File file ) when( mavenProject.getArtifact().getFile() ).thenReturn( file ); - return mavenProject; + try { + when(mavenProject.getRuntimeClasspathElements()).thenReturn(Arrays.asList(classPathElements)); + } catch (DependencyResolutionRequiredException e) { + fail("Encountered unexpected exception: " + e.getClass().getSimpleName() + ": " + e.getMessage()); + } + return mavenProject; } @Test public void finsishedShouldProduceMessageForClassesTotalNonZero() { - ArgumentCaptor logCaptor = ArgumentCaptor.forClass( CharSequence.class ); - - Log log = mock( Log.class ); - MinijarFilter m = new MinijarFilter( 1, 50, log ); m.finished(); @@ -153,10 +155,6 @@ public void finsishedShouldProduceMessageForClassesTotalNonZero() @Test public void finishedShouldProduceMessageForClassesTotalZero() { - ArgumentCaptor logCaptor = ArgumentCaptor.forClass( CharSequence.class ); - - Log log = mock( Log.class ); - MinijarFilter m = new MinijarFilter( 0, 0, log ); m.finished(); @@ -166,4 +164,29 @@ public void finishedShouldProduceMessageForClassesTotalZero() assertEquals( "Minimized 0 -> 0", logCaptor.getValue() ); } + + /** + * Check that the algorithm that removes services does not consider directories comming from the + * classpath as jar file candidates. + * + * @see https://issues.apache.org/jira/browse/MSHADE-366 + */ + @Test + public void remove_services_ignores_directories() throws Exception { + MavenProject mockedProject = mockProject(emptyFile, tempFolder.getRoot().getAbsolutePath()); + + new MinijarFilter(mockedProject, log); + + verify(log, never()).warn(logCaptor.capture()); + } + + @Test + public void remove_services_logs_ignored_items() throws Exception { + String classPathElementToIgnore = tempFolder.getRoot().getAbsolutePath(); + MavenProject mockedProject = mockProject(emptyFile, classPathElementToIgnore); + + new MinijarFilter(mockedProject, log); + + verify(log, times(1)).debug("Not a JAR file candidate. Ignoring classpath element '" + classPathElementToIgnore + "'."); + } }