Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[MSHADE-366] - "Access denied" during 'minimizeJar' #83

Closed
wants to merge 1 commit into from
Closed

[MSHADE-366] - "Access denied" during 'minimizeJar' #83

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
87 changes: 48 additions & 39 deletions src/main/java/org/apache/maven/plugins/shade/filter/MinijarFilter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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<JarEntry> 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<JarEntry> 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 + "'." );
}
}
}
Expand Down
75 changes: 49 additions & 26 deletions src/test/java/org/apache/maven/plugins/shade/filter/MinijarFilterTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<CharSequence> 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);
}

/**
Expand All @@ -64,12 +73,8 @@ public void testWithMockProject()
{
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 );

MinijarFilter mf = new MinijarFilter( mavenProject, log );

mf.finished();
Expand All @@ -84,14 +89,10 @@ public void testWithMockProject()
public void testWithPomProject()
throws IOException
{
ArgumentCaptor<CharSequence> 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();
Expand All @@ -105,7 +106,7 @@ public void testWithPomProject()

}

private MavenProject mockProject( File file )
private MavenProject mockProject( File file, String... classPathElements )
{
MavenProject mavenProject = mock( MavenProject.class );

Expand All @@ -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<CharSequence> logCaptor = ArgumentCaptor.forClass( CharSequence.class );

Log log = mock( Log.class );

MinijarFilter m = new MinijarFilter( 1, 50, log );

m.finished();
Expand All @@ -153,10 +155,6 @@ public void finsishedShouldProduceMessageForClassesTotalNonZero()
@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();
Expand All @@ -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 + "'.");
}
}