Skip to content
This repository has been archived by the owner on Jun 30, 2023. It is now read-only.

Commit

Permalink
Fix additional corruption with zips created on linux containing empty…
Browse files Browse the repository at this point in the history
… directories

This is similar to #36.

Go's zip library has some "interesting" behavior where if you call
zipWriter.CreateHeader with a file header that already contains
extra metadata encoding the last modified time for an empty directory,
it will append the current last modified time to the existing one
stored in the `Extra` field. This leads to corruption similar to #36
where MacOS will refuse to open the zip and `zipinfo -v` will show a
"There are an extra -xxx bytes preceding this file" warning.

I think the Go's library solution to this would be to use CreateRaw,
but we actually do want most of the logic that CreateHeader implements
(see #36 where we switched to CreateHeader). So instead, I just clear
the Extra field in the file header when copying over directories. This
means that directories will have their last modified time reset to the
current date. This is somewhat incorrect, but seems to me like a
reasonable compromise to me.
  • Loading branch information
ddworken committed Jan 5, 2022
1 parent 73b8540 commit 50e0590
Show file tree
Hide file tree
Showing 4 changed files with 11 additions and 1 deletion.
10 changes: 9 additions & 1 deletion jar/rewrite.go
Expand Up @@ -123,7 +123,15 @@ func Rewrite(w io.Writer, zr *zip.Reader) error {
copyFile:
if zipItem.Mode().IsDir() {
// Copy() only works on files, so manually create the directory entry
if _, err := zw.CreateHeader(&zipItem.FileHeader); err != nil {
dirHeader := zipItem.FileHeader
// Reset the Extra field which holds the OS-specific metadata that encodes the last
// modified time. This is technically incorrect because it means the mitigated
// zips that we create will have the last modified timestamp updated. But, if we don't
// do this we create invalid zips because `zw.CreateHeader` assumes that `Extra` is empty
// and always appends the modified time to the end of `Extra`. We don't use `zw.CreateRaw`
// because we want the rest of the logic that `zw.CreateHeader` provides.
dirHeader.Extra = make([]byte, 0)
if _, err := zw.CreateHeader(&dirHeader); err != nil {
return fmt.Errorf("failed to copy zip directory %s: %v", zipItem.Name, err)
}
} else {
Expand Down
2 changes: 2 additions & 0 deletions jar/rewrite_test.go
Expand Up @@ -363,6 +363,8 @@ func TestAutoMitigatedJarsAreCorrectlyFormed(t *testing.T) {
"log4j-core-2.1.jar",
"safe1.jar",
"safe1.signed.jar",
"emptydir.zip",
"emptydirs.zip",
}
for _, name := range testCases {
t.Run(name, func(t *testing.T) {
Expand Down
Binary file added jar/testdata/emptydir.zip
Binary file not shown.
Binary file added jar/testdata/emptydirs.zip
Binary file not shown.

0 comments on commit 50e0590

Please sign in to comment.