Skip to content

Support the Info-ZIP Unicode Path Extra Field. #41

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.

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

Merged
merged 3 commits into from
May 26, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@
<dependency>
<groupId>org.codehaus.plexus</groupId>
<artifactId>plexus-utils</artifactId>
<version>3.0.23</version>
<version>3.0.24</version>
</dependency>
<dependency>
<groupId>org.codehaus.plexus</groupId>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
* limitations under the License.
*/

import java.io.InputStream;
import java.io.IOException;
import java.util.Map;
import java.util.Properties;
import java.util.jar.Attributes;
Expand All @@ -33,25 +33,33 @@ class JdkManifestFactory
public static java.util.jar.Manifest getDefaultManifest()
throws ArchiverException
{
final java.util.jar.Manifest defaultManifest = new java.util.jar.Manifest();
defaultManifest.getMainAttributes().putValue( "Manifest-Version", "1.0" );
try
{
final java.util.jar.Manifest defaultManifest = new java.util.jar.Manifest();
defaultManifest.getMainAttributes().putValue( "Manifest-Version", "1.0" );

String createdBy = "Plexus Archiver";
String createdBy = "Plexus Archiver";

InputStream inputStream = JdkManifestFactory.class.getResourceAsStream( "/META-INF/"
+ "maven/org.codehaus.plexus/plexus-archiver/pom.properties" );
Properties properties = PropertyUtils.loadProperties( inputStream );
if ( properties != null )
{
String plexusArchiverVersion = properties.getProperty( "version" );
if ( plexusArchiverVersion != null )
final Properties properties = PropertyUtils.loadProperties( JdkManifestFactory.class.getResourceAsStream(
"/META-INF/maven/org.codehaus.plexus/plexus-archiver/pom.properties" ) );

if ( properties != null )
{
createdBy += " " + plexusArchiverVersion;
String plexusArchiverVersion = properties.getProperty( "version" );
if ( plexusArchiverVersion != null )
{
createdBy += " " + plexusArchiverVersion;
}
}
}
defaultManifest.getMainAttributes().putValue( "Created-By", createdBy );

return defaultManifest;
defaultManifest.getMainAttributes().putValue( "Created-By", createdBy );

return defaultManifest;
}
catch ( final IOException e )
{
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What purpose serves this try catch block? No one throws an IOException. Not the PropertyUtils.

Copy link
Member

@michael-o michael-o May 26, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I consider the swallow here was intentional. The local was actually borrowed from other spots where the version is omitted of the pom.properties could not be read. This needs a rewrite now.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Am 05/27/16 um 00:38 schrieb Michael Osipov:

In
src/main/java/org/codehaus/plexus/archiver/jar/JdkManifestFactory.java
#41 (comment):

  •    return defaultManifest;
    
  •        defaultManifest.getMainAttributes().putValue( "Created-By", createdBy );
    
  •        return defaultManifest;
    
  •    }
    
  •    catch ( final IOException e )
    
  •    {
    

What purpose serves this try catch block? No one throws an
|IOException|. Not the |PropertyUtils|.

PropertyUtils started throwing the exception with the latest patch
release of 'plexus-utils'.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Am 05/27/16 um 00:57 schrieb Michael Osipov:

In
src/main/java/org/codehaus/plexus/archiver/jar/JdkManifestFactory.java
#41 (comment):

  •    return defaultManifest;
    
  •        defaultManifest.getMainAttributes().putValue( "Created-By", createdBy );
    
  •        return defaultManifest;
    
  •    }
    
  •    catch ( final IOException e )
    
  •    {
    

I consider the swallow here
codehaus-plexus/plexus-utils@ba1c194
was intentional. The local was actually borrowed from other spots where
the version is omitted of the |pom.properties| could not be read.

Just restored that behaviour. I even considered throwing an
AssertionError if the properties cannot be read. Omitting the version
shouldn't be an issue, of course.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Am 2016-05-27 um 02:51 schrieb Christian Schulte:

  •    return defaultManifest;
    
  •        defaultManifest.getMainAttributes().putValue( "Created-By", createdBy );
    
  •        return defaultManifest;
    
  •    }
    
  •    catch ( final IOException e )
    
  •    {
    

Am 05/27/16 um 00:57 schrieb Michael Osipov:

In
src/main/java/org/codehaus/plexus/archiver/jar/JdkManifestFactory.java
#41 (comment):

  •    return defaultManifest;
    
  •        defaultManifest.getMainAttributes().putValue( "Created-By", createdBy );
    
  •        return defaultManifest;
    
  •    }
    
  •    catch ( final IOException e )
    
  •    {
    

I consider the swallow here
codehaus-plexus/plexus-utils@ba1c194
was intentional. The local was actually borrowed from other spots where
the version is omitted of the |pom.properties| could not be read.

Just restored that behaviour. I even considered throwing an
AssertionError if the properties cannot be read. Omitting the version

I am not convinced that AssertionError is the right thing because
Errors are reverved for the VM. Please search for pom.properties in
maven-core (DefaultRepositorySystemSessionFactory and
DefaultRuntimeInformation). You will see the basic ida it was taken from.

throw new ArchiverException( "Failure reading default manifest.", e );
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is obviously a wrong message now because no default manifest is read here.

}
}

public static void merge( java.util.jar.Manifest target, java.util.jar.Manifest other, boolean overwriteMain )
Expand Down
35 changes: 21 additions & 14 deletions src/main/java/org/codehaus/plexus/archiver/jar/Manifest.java
Original file line number Diff line number Diff line change
Expand Up @@ -735,25 +735,32 @@ public Iterator<String> iterator()
public static Manifest getDefaultManifest()
throws ArchiverException
{
final Manifest defaultManifest = new Manifest();
defaultManifest.getMainAttributes().putValue( "Manifest-Version", "1.0" );
try
{
final Manifest defaultManifest = new Manifest();
defaultManifest.getMainAttributes().putValue( "Manifest-Version", "1.0" );

String createdBy = "Plexus Archiver";
String createdBy = "Plexus Archiver";

InputStream inputStream = Manifest.class.getResourceAsStream( "/META-INF/"
+ "maven/org.codehaus.plexus/plexus-archiver/pom.properties" );
Properties properties = PropertyUtils.loadProperties( inputStream );
if ( properties != null )
{
String plexusArchiverVersion = properties.getProperty( "version" );
if ( plexusArchiverVersion != null )
final Properties properties = PropertyUtils.loadProperties( Manifest.class.getResourceAsStream(
"/META-INF/maven/org.codehaus.plexus/plexus-archiver/pom.properties" ) );

if ( properties != null )
{
createdBy += " " + plexusArchiverVersion;
String plexusArchiverVersion = properties.getProperty( "version" );
if ( plexusArchiverVersion != null )
{
createdBy += " " + plexusArchiverVersion;
}
}
}
defaultManifest.getMainAttributes().putValue( "Created-By", createdBy );
defaultManifest.getMainAttributes().putValue( "Created-By", createdBy );

return defaultManifest;
return defaultManifest;
}
catch ( final IOException e )
{
throw new ArchiverException( "Failure reading default manifest.", e );
}
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import org.apache.commons.compress.archivers.zip.ZipEncoding;
import org.apache.commons.compress.archivers.zip.ZipEncodingHelper;
import org.apache.commons.compress.parallel.InputStreamSupplier;
import org.apache.commons.compress.utils.Charsets;
import org.codehaus.plexus.archiver.AbstractArchiver;
import org.codehaus.plexus.archiver.ArchiveEntry;
import org.codehaus.plexus.archiver.Archiver;
Expand All @@ -41,6 +42,7 @@
import java.io.InputStream;
import java.io.OutputStream;
import java.io.SequenceInputStream;
import java.nio.charset.Charset;
import java.util.Hashtable;
import java.util.Stack;
import java.util.concurrent.ExecutionException;
Expand Down Expand Up @@ -208,7 +210,6 @@ public boolean isFilesonly()
return doFilesonly;
}


protected void execute()
throws ArchiverException, IOException
{
Expand Down Expand Up @@ -315,8 +316,7 @@ private void createArchiveMain()
zipArchiveOutputStream =
new ZipArchiveOutputStream( bufferedOutputStream( fileOutputStream( zipFile, "zip" ) ) );
zipArchiveOutputStream.setEncoding( encoding );
zipArchiveOutputStream.setCreateUnicodeExtraFields(
ZipArchiveOutputStream.UnicodeExtraFieldPolicy.NOT_ENCODEABLE );
zipArchiveOutputStream.setCreateUnicodeExtraFields( this.getUnicodeExtraFieldPolicy() );
zipArchiveOutputStream.setMethod(
doCompress ? ZipArchiveOutputStream.DEFLATED : ZipArchiveOutputStream.STORED );

Expand All @@ -339,6 +339,45 @@ private void createArchiveMain()
success = true;
}

/**
* Gets the {@code UnicodeExtraFieldPolicy} to apply.
*
* @return {@link ZipArchiveOutputStream.UnicodeExtraFieldPolicy.NOT_ENCODEABLE}, if the effective encoding is
* UTF-8; {@link ZipArchiveOutputStream.UnicodeExtraFieldPolicy.ALWAYS}, if the effective encoding is not
* UTF-8.
*
* @see #getEncoding()
*/
private ZipArchiveOutputStream.UnicodeExtraFieldPolicy getUnicodeExtraFieldPolicy()
{
// Copied from ZipEncodingHelper.isUTF8()
String effectiveEncoding = this.getEncoding();

if ( effectiveEncoding == null )
{
effectiveEncoding = Charset.defaultCharset().name();
}

boolean utf8 = Charsets.UTF_8.name().equalsIgnoreCase( effectiveEncoding );

if ( !utf8 )
{
for ( String alias : Charsets.UTF_8.aliases() )
{
if ( alias.equalsIgnoreCase( effectiveEncoding ) )
{
utf8 = true;
break;
}
}
}

return utf8
? ZipArchiveOutputStream.UnicodeExtraFieldPolicy.NOT_ENCODEABLE
: ZipArchiveOutputStream.UnicodeExtraFieldPolicy.ALWAYS;

}

/**
* Add the given resources.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,19 +20,21 @@
import java.io.File;
import java.io.IOException;
import java.io.InputStream;
import java.io.UnsupportedEncodingException;
import java.net.URL;
import java.util.Date;
import java.util.Enumeration;
import javax.annotation.Nonnull;

import org.apache.commons.compress.archivers.zip.UnicodePathExtraField;
import org.apache.commons.compress.archivers.zip.ZipArchiveEntry;
import org.apache.commons.compress.archivers.zip.ZipFile;
import org.apache.commons.compress.utils.IOUtils;

import org.codehaus.plexus.archiver.AbstractUnArchiver;
import org.codehaus.plexus.archiver.ArchiverException;
import org.codehaus.plexus.components.io.resources.PlexusIoResource;

import javax.annotation.Nonnull;

/**
* @author <a href="mailto:[email protected]">Emmanuel Venisse</a>
* @version $Id$
Expand Down Expand Up @@ -84,7 +86,20 @@ private static class ZipEntryFileInfo

public String getName()
{
return zipEntry.getName();
try
{
final UnicodePathExtraField unicodePath =
(UnicodePathExtraField) zipEntry.getExtraField( UnicodePathExtraField.UPATH_ID );

return unicodePath != null
? new String( unicodePath.getUnicodeName(), "UTF-8" )
: zipEntry.getName();

}
catch ( final UnsupportedEncodingException e )
{
throw new AssertionError( e );
}
}

public boolean isDirectory()
Expand Down Expand Up @@ -141,27 +156,30 @@ protected void execute()
InputStream in = null;
try
{
zf = new org.apache.commons.compress.archivers.zip.ZipFile( getSourceFile(), encoding );
final Enumeration e = zf.getEntries();
zf = new org.apache.commons.compress.archivers.zip.ZipFile( getSourceFile(), encoding, true );
final Enumeration e = zf.getEntriesInPhysicalOrder();
while ( e.hasMoreElements() )
{
final ZipArchiveEntry ze = (ZipArchiveEntry) e.nextElement();
final ZipEntryFileInfo fileInfo = new ZipEntryFileInfo( zf, ze );
if ( isSelected( ze.getName(), fileInfo ) )
if ( isSelected( fileInfo.getName(), fileInfo ) )
{
in = zf.getInputStream( ze );
extractFileIfIncluded(getSourceFile(), getDestDirectory(), in, ze.getName(),
new Date(ze.getTime()), ze.isDirectory(), ze.getUnixMode() != 0 ? ze.getUnixMode() : null,
resolveSymlink( zf, ze ) );
in = zf.getInputStream( ze );

extractFileIfIncluded( getSourceFile(), getDestDirectory(), in, fileInfo.getName(),
new Date( ze.getTime() ), ze.isDirectory(),
ze.getUnixMode() != 0 ? ze.getUnixMode() : null,
resolveSymlink( zf, ze ) );

in.close();
in = null;
}

}
}
}

getLogger().debug( "expand complete" );
zf.close();
zf = null;

getLogger().debug( "expand complete" );
}
catch ( final IOException ioe )
{
Expand Down Expand Up @@ -197,9 +215,9 @@ protected void execute( final String path, final File outputDirectory )
InputStream in = null;
try
{
zipFile = new org.apache.commons.compress.archivers.zip.ZipFile( getSourceFile(), encoding );
zipFile = new org.apache.commons.compress.archivers.zip.ZipFile( getSourceFile(), encoding, true );

final Enumeration e = zipFile.getEntries();
final Enumeration e = zipFile.getEntriesInPhysicalOrder();

while ( e.hasMoreElements() )
{
Expand All @@ -213,9 +231,12 @@ protected void execute( final String path, final File outputDirectory )
if ( ze.getName().startsWith( path ) )
{
in = zipFile.getInputStream( ze );

extractFileIfIncluded( getSourceFile(), outputDirectory, in,
ze.getName(), new Date( ze.getTime() ), ze.isDirectory(),
ze.getUnixMode() != 0 ? ze.getUnixMode() : null, resolveSymlink( zipFile, ze ) );
ze.getUnixMode() != 0 ? ze.getUnixMode() : null,
resolveSymlink( zipFile, ze ) );

in.close();
in = null;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
import org.codehaus.plexus.PlexusTestCase;
import org.codehaus.plexus.archiver.Archiver;
import org.codehaus.plexus.archiver.util.DefaultArchivedFileSet;
import org.codehaus.plexus.components.io.attributes.Java7AttributeUtils;
import org.codehaus.plexus.components.io.functions.InputStreamTransformer;
import org.codehaus.plexus.components.io.resources.PlexusIoResource;

Expand All @@ -12,6 +11,9 @@
import java.io.IOException;
import java.io.InputStream;
import java.nio.charset.Charset;
import org.codehaus.plexus.archiver.util.ArchiveEntryUtils;
import org.codehaus.plexus.logging.Logger;
import org.codehaus.plexus.logging.console.ConsoleLogger;

public class DirectoryArchiverUnpackJarTest
extends PlexusTestCase
Expand Down Expand Up @@ -52,12 +54,13 @@ public void test_dependency_sets_depSet_unpacked_rdonly()
archiver.createArchive();
assertTrue( new File( "target/depset_unpack/child-1/META-INF/MANIFEST.MF" ).exists() );

final Logger logger = new ConsoleLogger( Logger.LEVEL_DEBUG, this.getClass().getName() );

// make them writeable or mvn clean will fail
Java7AttributeUtils.chmod( new File("target/depset_unpack/child-1/META-INF"), 0777);
Java7AttributeUtils.chmod( new File("target/depset_unpack/child-1/META-INF/maven"), 0777);
Java7AttributeUtils.chmod( new File("target/depset_unpack/child-1/META-INF/maven/test"), 0777);
Java7AttributeUtils.chmod( new File("target/depset_unpack/child-1/META-INF/maven/test/child1"), 0777);
Java7AttributeUtils.chmod( new File("target/depset_unpack/child-1/assembly-resources"), 0777);
ArchiveEntryUtils.chmod( new File( "target/depset_unpack/child-1/META-INF" ), 0777, logger );
ArchiveEntryUtils.chmod( new File( "target/depset_unpack/child-1/META-INF/maven" ), 0777, logger );
ArchiveEntryUtils.chmod( new File( "target/depset_unpack/child-1/META-INF/maven/test" ), 0777, logger );
ArchiveEntryUtils.chmod( new File( "target/depset_unpack/child-1/META-INF/maven/test/child1" ), 0777, logger );
ArchiveEntryUtils.chmod( new File( "target/depset_unpack/child-1/assembly-resources" ), 0777, logger );
}
}