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.

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

Zip files are incompatible with Java's java.util.zip #1

Open
sveniu opened this issue Oct 12, 2020 · 5 comments
Open

Zip files are incompatible with Java's java.util.zip #1

sveniu opened this issue Oct 12, 2020 · 5 comments

Comments

@sveniu
Copy link

sveniu commented Oct 12, 2020

The zip files produced by this patch are causing incompatibility issues that don't seem to be very easy to work around.

java.util.zip.ZipException: only DEFLATED entries can have EXT descriptor
        at java.base/java.util.zip.ZipInputStream.readLOC(ZipInputStream.java:311)
        at java.base/java.util.zip.ZipInputStream.getNextEntry(ZipInputStream.java:123)
        at com.meh.test.UnzipFiles.unzip(UnzipFiles.java:30)
        at com.meh.test.UnzipFiles.main(UnzipFiles.java:17)

The combination of Go's streaming (i.e. writing ext data descriptors after each file entry) and use of STORE is not supported by Java.

More details: hashicorp#47 (review)

@sveniu
Copy link
Author

sveniu commented Oct 13, 2020

I put the test zip in aws/containers-roadmap#1112, and just now edited it to include an unzip program and jre version info. It's at the bottom of the issue text.

@sveniu
Copy link
Author

sveniu commented Oct 13, 2020

Oh, and the zip was produced by

provider "archive" {
  # Use the patched provider.
  version = "1.2.0-4-g347a1af"
}

data "archive_file" "test" {
  type        = "zip"
  source_file = "${path.module}/src/test1.txt"
  output_path = "${path.module}/test.zip"

  normalize_files_metadata = true
}

@64kramsystem
Copy link
Owner

64kramsystem commented Nov 10, 2020

@sveniu So! Many thanks for the plethora of information (and sorry for the late reply) 🙂

First of all, there is a working solution.

Now, there is a bigger picture discussion that needs to be taken into account.

First, regarding the solution itself, it's straightforward to integrate the zipper library you've referenced. In the basic implementation, terraform-provider-archive-dev creates the zip files as it currently does, with a temporary filename, then process it and outputs in the final form.

Since the zipper processing logic works in memory, a second iteration could simply create the zip file in-memory, process it, and output it. In this case, I'd need to have a look at the APIs, since I'm not a Golang programmer, but my rough guess is that this shouldn't be a problem.

I didn't do extensive testing, but I've written a simple Golang compressor, with integrated the zipper, and tested both on the file you've provided and on another file, and the Java decompression works fine. Of course, I can't confirm this is stable without further tests.

Now, the discussion is ultimately about how to design/distribute the change.

Assuming that the patch is stable, it should decided if:

  1. another flag should be used for this special processing,
  2. if the normalization should always be processed.

I think option 2 is fine. I don't see any use case where a user would bother the flexibility given by option 1.

Regarding the distribution, we have freedom here. The TF provider maintainers (there's really no nicer way to put this) simply don't care about this PR, so there are no restrictions here - if you think the processing should be added to the plugin, I'll do it, test it, release it, etc.

Thanks for the participation 🙂

@64kramsystem
Copy link
Owner

Source code of the program I've performed the tests with. Nothing special, just adding them for reference.

unzip.java.txt
zip.go.txt

@64kramsystem
Copy link
Owner

I'm bringing this to closure :)

In my previous reply, I've explained the solution (and tested it). I did not integrate because the maintainers are not showing interest in the topic.

This fork, and the related work, sit therefore in a strange place between a contribution for an upstream project, and an internal one. From the upstreaming perspective, any work is a waste, since it's factually not considered. From our internal tooling perspective, any work is also a waste, since everything works for us already (and TF development in not in our focus).

If the upstream project had interest, I'd certainly improve the PR, but this is not the case. Therefore, I've officially decided I'm done with any terraform-provider-archive-dev.

That said:

  • I'm keeping this open for reference, in case somebody would like to understand the issue and the solution
  • I thank you again for the information provided, that I indeed used for testing the problem and the solution :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants