This is an automated email from the ASF dual-hosted git repository.
gnodet pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/maven-shade-plugin.git
The following commit(s) were added to refs/heads/master by this push:
new 41bd72f [MSHADE-366] "Access denied" during 'minimizeJar' (#161)
41bd72f is described below
commit 41bd72f0fd0195713f4a4b73af1184ec8a6cc0a3
Author: Guillaume Nodet <gno...@gmail.com>
AuthorDate: Thu Oct 20 22:35:52 2022 +0200
[MSHADE-366] "Access denied" during 'minimizeJar' (#161)
* [MSHADE-366] - "Access denied" during 'minimizeJar'
Now ignoring directories when scanning the classpath for services.
* [MSHADE-366] Refactor fix by @JanMosigItemis from #83
- Simplify Jan's solution from #83 in order to use 'continue' instead of
nested 'if-else'.
- Factor out two helper methods from 'removeServices', because that
method was way too big to still be readable.
- DRY-refactor Jan's new test cases into one checking two conditions.
* Another attempt to clarify the problem
- do not ignore directories, print a warning as before
- ignore the project's build output directory which is always returned by
getRuntimeClassPathElements()
* Fix the test to work on all platforms, irrespective of the actual
exception sent by the JDK
Co-authored-by: Jan Mosig <jan.mo...@itemis.de>
Co-authored-by: Alexander Kriegisch <alexan...@kriegisch.name>
---
.../maven/plugins/shade/filter/MinijarFilter.java | 121 +++++++++++++--------
.../plugins/shade/filter/MinijarFilterTest.java | 86 +++++++++++----
2 files changed, 137 insertions(+), 70 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 818339e..a996bd4 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
@@ -129,58 +129,22 @@ public class MinijarFilter
neededClasses.removeAll( removable );
try
{
+ // getRuntimeClasspathElements returns a list of
+ // - the build output directory
+ // - all the paths to the dependencies' jars
+ // We thereby need to ignore the build directory because we
don't want
+ // to remove anything from it, as it's the starting point of
the
+ // minification process.
for ( final String fileName :
project.getRuntimeClasspathElements() )
{
- try ( final JarFile jar = new JarFile( fileName ) )
+ // Ignore the build directory from this project
+ if ( fileName.equals(
project.getBuild().getOutputDirectory() ) )
{
- for ( final Enumeration<JarEntry> entries =
jar.entries(); entries.hasMoreElements(); )
- {
- final JarEntry jarEntry = entries.nextElement();
- if ( jarEntry.isDirectory() ||
!jarEntry.getName().startsWith( "META-INF/services/" ) )
- {
- 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 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
- }
- }
- catch ( final IOException e )
- {
- log.warn( e.getMessage() );
- }
- }
+ continue;
}
- catch ( final IOException e )
+ if ( removeServicesFromJar( cp, neededClasses, fileName ) )
{
- log.warn( e.getMessage() );
+ repeatScan = true;
}
}
}
@@ -192,6 +156,69 @@ public class MinijarFilter
while ( repeatScan );
}
+ private boolean removeServicesFromJar( Clazzpath cp, Set<Clazz>
neededClasses, String fileName )
+ {
+ boolean repeatScan = false;
+ try ( final JarFile jar = new JarFile( fileName ) )
+ {
+ for ( final Enumeration<JarEntry> entries = jar.entries();
entries.hasMoreElements(); )
+ {
+ final JarEntry jarEntry = entries.nextElement();
+ if ( jarEntry.isDirectory() || !jarEntry.getName().startsWith(
"META-INF/services/" ) )
+ {
+ 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 configFileReader = new
BufferedReader(
+ new InputStreamReader( jar.getInputStream( jarEntry ),
UTF_8 ) ) )
+ {
+ // check whether the found classes use services in turn
+ repeatScan = scanServiceProviderConfigFile( cp,
configFileReader );
+ }
+ catch ( final IOException e )
+ {
+ log.warn( e.getMessage() );
+ }
+ }
+ }
+ catch ( final IOException e )
+ {
+ log.warn( "Not a JAR file candidate. Ignoring classpath element '"
+ fileName + "' (" + e + ")." );
+ }
+ return repeatScan;
+ }
+
+ private boolean scanServiceProviderConfigFile( Clazzpath cp,
BufferedReader configFileReader ) throws IOException
+ {
+ boolean serviceClassFound = false;
+ for ( String line = configFileReader.readLine(); line != null; line =
configFileReader.readLine() )
+ {
+ 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 );
+ serviceClassFound = true;
+ }
+ return serviceClassFound;
+ }
+
private void removeClass( final Clazz clazz )
{
removable.remove( clazz );
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 bfbaee2..25be4d8 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
@@ -19,7 +19,10 @@ package org.apache.maven.plugins.shade.filter;
* under the License.
*/
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.startsWith;
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.times;
@@ -27,15 +30,23 @@ import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
import java.io.File;
+import java.io.FileOutputStream;
import java.io.IOException;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
import java.util.Set;
import java.util.TreeSet;
+import java.util.jar.JarOutputStream;
import org.apache.maven.artifact.Artifact;
import org.apache.maven.artifact.DefaultArtifact;
+import org.apache.maven.artifact.DependencyResolutionRequiredException;
+import org.apache.maven.model.Build;
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;
@@ -43,16 +54,25 @@ import org.mockito.ArgumentCaptor;
public class MinijarFilterTest
{
+ @Rule
+ public TemporaryFolder tempFolder =
TemporaryFolder.builder().assureDeletion().build();
+
+ private File outputDirectory;
private File emptyFile;
+ private File jarFile;
+ private Log log;
+ private ArgumentCaptor<CharSequence> logCaptor;
@Before
public void init()
throws IOException
{
- TemporaryFolder tempFolder = new TemporaryFolder();
- tempFolder.create();
+ this.outputDirectory = tempFolder.newFolder();
this.emptyFile = tempFolder.newFile();
-
+ this.jarFile = tempFolder.newFile();
+ new JarOutputStream( new FileOutputStream( this.jarFile ) ).close();
+ this.log = mock(Log.class);
+ logCaptor = ArgumentCaptor.forClass(CharSequence.class);
}
/**
@@ -64,11 +84,7 @@ public class MinijarFilterTest
{
assumeFalse( "Expected to run under JDK8+",
System.getProperty("java.version").startsWith("1.7") );
- ArgumentCaptor<CharSequence> logCaptor = ArgumentCaptor.forClass(
CharSequence.class );
-
- MavenProject mavenProject = mockProject( emptyFile );
-
- Log log = mock( Log.class );
+ MavenProject mavenProject = mockProject( outputDirectory, emptyFile );
MinijarFilter mf = new MinijarFilter( mavenProject, log );
@@ -84,14 +100,10 @@ public class MinijarFilterTest
public void testWithPomProject()
throws IOException
{
- ArgumentCaptor<CharSequence> logCaptor = ArgumentCaptor.forClass(
CharSequence.class );
-
// project with pom packaging and no artifact.
- MavenProject mavenProject = mockProject( null );
+ MavenProject mavenProject = mockProject( outputDirectory, null );
mavenProject.setPackaging( "pom" );
- Log log = mock( Log.class );
-
MinijarFilter mf = new MinijarFilter( mavenProject, log );
mf.finished();
@@ -105,7 +117,7 @@ public class MinijarFilterTest
}
- private MavenProject mockProject( File file )
+ private MavenProject mockProject( File outputDirectory, File file,
String... classPathElements )
{
MavenProject mavenProject = mock( MavenProject.class );
@@ -129,17 +141,29 @@ public class MinijarFilterTest
when( mavenProject.getArtifact().getFile() ).thenReturn( file );
- return mavenProject;
+ Build build = new Build();
+ build.setOutputDirectory( outputDirectory.toString() );
+
+ List<String> classpath = new ArrayList<>();
+ classpath.add( outputDirectory.toString() );
+ if ( file != null )
+ {
+ classpath.add(file.toString());
+ }
+ classpath.addAll( Arrays.asList( classPathElements ) );
+ when( mavenProject.getBuild() ).thenReturn( build );
+ try {
+
when(mavenProject.getRuntimeClasspathElements()).thenReturn(classpath);
+ } catch (DependencyResolutionRequiredException e) {
+ fail("Encountered unexpected exception: " +
e.getClass().getSimpleName() + ": " + e.getMessage());
+ }
+ return mavenProject;
}
@Test
public void finsishedShouldProduceMessageForClassesTotalNonZero()
{
- ArgumentCaptor<CharSequence> logCaptor = ArgumentCaptor.forClass(
CharSequence.class );
-
- Log log = mock( Log.class );
-
MinijarFilter m = new MinijarFilter( 1, 50, log );
m.finished();
@@ -153,10 +177,6 @@ public class MinijarFilterTest
@Test
public void finishedShouldProduceMessageForClassesTotalZero()
{
- ArgumentCaptor<CharSequence> logCaptor = ArgumentCaptor.forClass(
CharSequence.class );
-
- Log log = mock( Log.class );
-
MinijarFilter m = new MinijarFilter( 0, 0, log );
m.finished();
@@ -166,4 +186,24 @@ public class MinijarFilterTest
assertEquals( "Minimized 0 -> 0", logCaptor.getValue() );
}
+
+ /**
+ * Verify that directories are ignored when scanning the classpath for
JARs containing services,
+ * but warnings are logged instead
+ *
+ * @see <a
href="https://issues.apache.org/jira/browse/MSHADE-366";>MSHADE-366</a>
+ */
+ @Test
+ public void removeServicesShouldIgnoreDirectories() throws Exception {
+ String classPathElementToIgnore =
tempFolder.newFolder().getAbsolutePath();
+ MavenProject mockedProject = mockProject( outputDirectory, jarFile,
classPathElementToIgnore );
+
+ new MinijarFilter(mockedProject, log);
+
+ verify( log, times( 1 ) ).warn( logCaptor.capture() );
+
+ assertThat( logCaptor.getValue().toString(), startsWith(
+ "Not a JAR file candidate. Ignoring classpath element '" +
classPathElementToIgnore + "' (" ) );
+ }
+
}
