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

Jump to bottom

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

Merged
merged 4 commits into from Oct 20, 2022
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
71c7a82
[MSHADE-366] - "Access denied" during 'minimizeJar'
JanMosigItemis Jan 28, 2021
6bf6662
[MSHADE-366] Refactor fix by @JanMosigItemis from #83
kriegaex Jul 3, 2021
ed8ec8e
Another attempt to clarify the problem
gnodet Oct 20, 2022
5213c6b
Fix the test to work on all platforms, irrespective of the actual exc…
gnodet Oct 20, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
121 changes: 74 additions & 47 deletions src/main/java/org/apache/maven/plugins/shade/filter/MinijarFilter.java
View file
Open in desktop
Expand Up @@ -129,58 +129,22 @@ private void removeServices( final MavenProject project, final Clazzpath cp )
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;
}
}
}
Expand All @@ -192,6 +156,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( "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 );
Expand Down
86 changes: 63 additions & 23 deletions src/test/java/org/apache/maven/plugins/shade/filter/MinijarFilterTest.java
View file
Open in desktop
Expand Up @@ -19,40 +19,60 @@
* 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;
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;

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);
}

/**
Expand All @@ -64,11 +84,7 @@ 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 );
MavenProject mavenProject = mockProject( outputDirectory, emptyFile );

MinijarFilter mf = new MinijarFilter( mavenProject, log );

Expand All @@ -84,14 +100,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 mavenProject = mockProject( outputDirectory, null );
mavenProject.setPackaging( "pom" );

Log log = mock( Log.class );

MinijarFilter mf = new MinijarFilter( mavenProject, log );

mf.finished();
Expand All @@ -105,7 +117,7 @@ public void testWithPomProject()

}

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

Expand All @@ -129,17 +141,29 @@ private MavenProject mockProject( File file )

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();
Expand All @@ -153,10 +177,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 +186,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.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 + "' (" ) );
}

}