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' (supersedes #83) #104

Closed
wants to merge 2 commits into from
Closed

[MSHADE-366] - "Access denied" during 'minimizeJar' (supersedes #83) #104

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e608482
[MSHADE-366] - "Access denied" during 'minimizeJar'
JanMosigItemis Jan 28, 2021
faa97df
[MSHADE-366] Refactor fix by @JanMosigItemis from #83
kriegaex Jul 3, 2021
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
115 changes: 68 additions & 47 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 @@ -131,56 +131,14 @@ private void removeServices( final MavenProject project, final Clazzpath cp )
{
for ( final String fileName : project.getRuntimeClasspathElements() )
{
try ( final JarFile jar = new JarFile( fileName ) )
if ( new File( fileName ).isDirectory() )
{
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() );
}
}
log.debug( "Not a JAR file candidate. Ignoring classpath element '" + fileName + "'." );
continue;
}
catch ( final IOException e )
if ( removeServicesFromJar( cp, neededClasses, fileName ) )
{
log.warn( e.getMessage() );
repeatScan = true;
}
}
}
Expand All @@ -192,6 +150,69 @@ private void removeServices( final MavenProject project, final Clazzpath cp )
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( e.getMessage() );
}
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 );
Expand Down
60 changes: 39 additions & 21 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.Arrays;
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;

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,24 @@ public void finishedShouldProduceMessageForClassesTotalZero()
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.getRoot().getAbsolutePath();
MavenProject mockedProject = mockProject(emptyFile, classPathElementToIgnore);

new MinijarFilter(mockedProject, log);

verify(log, never()).warn(logCaptor.capture());
verify(log, times(1)).debug(
"Not a JAR file candidate. Ignoring classpath element '" + classPathElementToIgnore + "'."
);
}

}