[MSHADE-417] Fix null bytes appended to small files by maven-shade-plugin

[jira-importer]

Max Zerzouri opened MSHADE-417 and commented

Version 3.3.0 of maven shade plugin seems to append "\x00" bytes to a file if it's less than 4 bytes:

$ echo -n a > src/main/resources/test.txt
$ mvn clean install
...
$ bsdtar -xOf target/original-shadetest-0.0.0.jar test.txt | xxd
00000000: 61                                       a
$ bsdtar -xOf target/shadetest-0.0.0.jar test.txt | xxd
00000000: 6100 0000                                a...

I've attached a basic pom.xml that triggers this. This doesn't occur in the previous version, 3.2.4.

---

Affects: 3.3.0

Attachments:

• original-shadetest-0.0.0.jar (1.55 kB)
• pom.xml (979 bytes)
• shadetest-0.0.0.jar (1.69 kB)
• test.txt (1 bytes)

Issue Links:

• MSHADE-378 Shade plugin changes the compression level of nested jar entries
  ("is broken by")

Remote Links:

• GitHub Pull Request #140
  
• GitHub Pull Request #160
  

3 votes, 7 watchers

[jira-importer]

Michael Osipov commented

Interesting, this doesn't look look. I trust libarchive(3) doing the right thing. There is a zipdetails(3) in Perl which should show more. Can you upload both files?

[jira-importer]

Max Zerzouri commented

Sure. I've uploaded those files now.
Not familiar with zipdetails, but here's what it shows for the test.txt entry:

04E7 CENTRAL HEADER #3     02014B50                              
04EB Created Zip Spec      14 '2.0'
04EC Created OS            00 'MS-DOS'
04ED Extract Zip Spec      14 '2.0'
04EE Extract OS            00 'MS-DOS'
04EF General Purpose Flag  0808    
     [Bits 1-2]            0 'Normal Compression'
     [Bit  3]              1 'Streamed'
     [Bit 11]              1 'Language Encoding' 
04F1 Compression Method    0008 'Deflated'
04F3 Last Mod Time         547FB3CA 'Thu Mar 31 22:30:20 2022'
04F7 CRC                   A2DE4F7A       
04FB Compressed Length     00000006                           
04FF Uncompressed Length   00000004
0503 Filename Length       0008    
0505 Extra Length          0000    
0507 Comment Length        0000
0509 Disk Start            0000
050B Int File Attributes   0000
     [Bit 0]               0 'Binary Data'
050D Ext File Attributes   00000000
0511 Local Header Offset   000000E8       
0515 Filename              'test.txt'

The regular zip handling used by the normal classloader in OpenJDK also sees the zero bytes, which is how we noticed the issue (because we had some small JSON files in the classpath containing just the string {}, which failed to parse due to trailing zero bytes).

[jira-importer]

Michael Osipov commented

Trying to reproduce at the moment...

[jira-importer]

Michael Osipov commented

I can confirm this even with 3.3.1-SNAPSHOT:

$ ~/.local/bin/diffoscope original-shadetest-0.0.0.jar shadetest-0.0.0.jar
--- original-shadetest-0.0.0.jar
+++ shadetest-0.0.0.jar
├── zipinfo {}
│ @@ -1,10 +1,10 @@
│ -Zip file size: 1580 bytes, number of entries: 8
│ -drwxr-xr-x  2.0 unx        0 b- stor 22-Mar-31 22:30 META-INF/
│ --rw-r--r--  2.0 unx      126 b- defN 22-Mar-31 22:30 META-INF/MANIFEST.MF
│ --rw-r--r--  2.0 unx        1 b- defN 22-Mar-31 22:30 test.txt
│ -?rwsrwsrwt  2.0 unx        0 b- stor 22-Mar-31 22:30 META-INF/maven/
│ -?rwsrwsrwt  2.0 unx        0 b- stor 22-Mar-31 22:30 META-INF/maven/shadetest/
│ -?rwsrwsrwt  2.0 unx        0 b- stor 22-Mar-31 22:30 META-INF/maven/shadetest/shadetest/
│ --rw-r--r--  2.0 unx      979 b- defN 22-Mar-31 22:29 META-INF/maven/shadetest/shadetest/pom.xml
│ --rw-r--r--  2.0 unx      104 b- defN 22-Mar-31 22:30 META-INF/maven/shadetest/shadetest/pom.properties
│ -8 files, 1210 bytes uncompressed, 544 bytes compressed:  55.0%
│ +Zip file size: 1727 bytes, number of entries: 8
│ +-rw----     2.0 fat        0 bX defN 22-Mar-31 22:30 META-INF/
│ +-rw----     2.0 fat      126 bl defN 22-Mar-31 22:30 META-INF/MANIFEST.MF
│ +-rw----     2.0 fat        4 bl defN 22-Mar-31 22:30 test.txt
│ +-rw----     2.0 fat        0 bl defN 22-Mar-31 22:29 META-INF/maven/
│ +-rw----     2.0 fat        0 bl defN 22-Mar-31 22:29 META-INF/maven/shadetest/
│ +-rw----     2.0 fat        0 bl defN 22-Mar-31 22:29 META-INF/maven/shadetest/shadetest/
│ +-rw----     2.0 fat      979 bl defN 22-Mar-31 22:29 META-INF/maven/shadetest/shadetest/pom.xml
│ +-rw----     2.0 fat      104 bl defN 22-Mar-31 22:30 META-INF/maven/shadetest/shadetest/pom.properties
│ +8 files, 1213 bytes uncompressed, 555 bytes compressed:  54.2%
├── filetype from file(1)
│ @@ -1 +1 @@
│ -Zip archive data, at least v1.0 to extract, compression method=store
│ +Java archive data (JAR)
├── test.txt
│ @@ -1 +1 @@
│ -00000000: 61                                       a
│ +00000000: 6100 0000                                a...

The new zip file is logically different, regardless of your wrenched text file. That is UGLY. Let me have a deeper look into that.

[jira-importer]

Michael Osipov commented

The first and foremost problem is that all packaging plugins use Maven Archiver/Plexus Archiver/Commons Compress which MSHADE uses Java's ZipFile. Hervé Boutemy, any idea why we have this inconsistency?

[jira-importer]

Max Zerzouri commented

I should probably have mentioned that the zero bytes issue doesn't occur in the previous version (3.2.4). 3.2.4 produces the same structural differences, but doesn't pad small files with zero bytes.

[jira-importer]

Michael Osipov commented

c798d01 is the first bad commit

commit c798d01138e9fecdd6422a2a8acce22ca8987924
Author: jenrryyou <flyaway.yuan@qq.com>
Date:   Sun Nov 22 23:17:56 2020 +0800

    [MSHADE-378] Shade plugin changes the compression level of nested jar… (#73)

    * [MSHADE-378] Shade plugin changes the compression level of nested jar entries

    * [MSHADE-378] Shade plugin changes the compression level of nested jar entries

    Co-authored-by: shaoyao <jeremy.yzy@alibaba-inc.com>

 .../apache/maven/plugins/shade/DefaultShader.java  | 107 +++++++++++++++++++--
 .../maven/plugins/shade/DefaultShaderTest.java     |  67 +++++++++++++
 2 files changed, 165 insertions(+), 9 deletions(-)

[jira-importer]

Michael Osipov commented

#73 and the merge did not care to take care of the ticket. It is still open and unassigned to a fix version.

[jira-importer]

Hervé Boutemy commented

The first and foremost problem is that all packaging plugins use Maven Archiver/Plexus Archiver/Commons Compress which MSHADE uses Java's ZipFile.

Michael Osipov I don't get why this is a problem nor how it is relevant to the issue reported

[jira-importer]

Hervé Boutemy commented

rmannibucau it seems you have merged PR #73 but not updated MSHADE-378 : was it intentional?

and it seems it creates a strange behaviour, adding null bytes: do you have any idea?

[jira-importer]

Max Zerzouri commented

Looking at the changes in that PR, I would guess it's because of these lines in ZipHeaderPeekInputStream.hasZipHeader:

final byte[] header = new byte[HEADER_LEN];
super.read( header, 0, HEADER_LEN );
super.unread( header );

It always pushes back the entirety of that byte array (4 bytes, HEADER_LEN), even though super.read could have read less than 4 bytes. The remaining bytes in the header array will naturally be 0. It should probably be looking at the return value of super.read to know how many bytes to actually unread.

[jira-importer]

Romain Manni-Bucau commented

Hervé Boutemy , yes I don't have perms to do that (mentionned it a few times on slack, guess it is a bad username - romain.manni-bucau is the right one?).

Not sure about your use case for null bytes, do you have some sample? Max analyzis is right but also means we create a shade of nothing which sounds like a project setup error.

[jira-importer]

Hervé Boutemy commented

I don't know where "on Slack" it has been reported, but merging a PR and not asking others help for the Jira part is not really a good move: IIUC, I should close MSHADE-378 as fixed in 3.3.0?

 

then on you not having perms: I see there are 2 profiles

• the profile that is member of maven, but also members and a few other ones is https://issues.apache.org/jira/secure/ViewProfile.jspa?name=rmannibucau that you used until 2015, and matches your Apache id
• you seem to be now using https://issues.apache.org/jira/secure/ViewProfile.jspa?name=romain.manni-bucau which is an external user

You should define if you recover your initial account that matches your Apache id or really want to switch to the new one

then if you want to switch, you'll need to ask INFRA about it

(all that does not really have to do anything with Maven...)

[jira-importer]

Romain Manni-Bucau commented

Yep please close as fixed. I used romain.manni-bucau since a bit more for most asf project, only openejb used rmannibucau before switching, not sure where the assumption the same name is used on both systems comes from but happy to fix it if it is a missed update sby can point out.

[jira-importer]

Michael Osipov commented

Hervé Boutemy, it is not related to this bug. The point I am trying to make it that:

• The library used is inconsistent with other packaging plugins,
• It changes a lot of metadata and this I consider plain wrong.