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

Support for Traditional Encryption (ZipCrypto) + Tests #209

Merged
merged 13 commits into from Jan 17, 2015

Conversation

johnnyshields
Copy link
Contributor

This PR adds support for traditional encryption. The interface is:

      Zip::OutputStream.write_buffer(::StringIO.new(''), Zip::TraditionalEncrypter.new('password')) do |out|
        out.put_next_entry("my_file.txt")
        out.write my_data
      end.string

This preserves backwards compatibility with existing rubyzip, however I'd suggest that it might be cleaner to do something like an options-hash based interface in future versions.

The PR contains unit and integration tests. It seems that InfoZip uses a different crc32 function from Zlib, so we had to include a zip file generated with archive-zip into the repo (couldn't get InfoZip-based generators to work.)

There is a separate PR for AES encryption (#179) which is read only. We believe our code structure is a cleaner one which allows for both read and write and it should be fairly straightforward to adapt #179 into our structure.

Big thanks to @matsu911 who did the work.

@coveralls
Copy link

Coverage Status

Coverage increased (+1.35%) when pulling 71225dc on johnnyshields:master into deeefa1 on rubyzip:master.

@simonoff
Copy link
Member

@johnnyshields Hi, can you look on 1.9.3 build? It failed. If you will be not possible to fix it would be better add fixes for gemfile to set required version of ruby to 2.0.0 or more.

@coveralls
Copy link

Coverage Status

Coverage increased (+1.5%) when pulling a54191f on johnnyshields:master into deeefa1 on rubyzip:master.

@johnnyshields
Copy link
Contributor Author

@matsu911 fixed it--thanks! Travis is now green

simonoff added a commit that referenced this pull request Jan 17, 2015
Support for Traditional Encryption (ZipCrypto) + Tests
@simonoff simonoff merged commit 9e7af06 into rubyzip:master Jan 17, 2015
@johnnyshields
Copy link
Contributor Author

@simonoff thanks for merging and @matsu911 thanks for the awesome work!

@simonoff
Copy link
Member

@johnnyshields n/p. But would be great to update README with info how to use this feature.

@johnnyshields
Copy link
Contributor Author

Yeah, I agree, but I think we should mark this feature as "experimental" for the time being. I personally am not 100% happy with the interface, it's harder to use than it should be. Here's what I mean:

  # current
  Zip::OutputStream.write_buffer(::StringIO.new(''), Zip::TraditionalEncrypter.new('foobar')) 

  # better (but breaks existing interface)
  Zip::OutputStream.write_buffer(password: 'foobar', encryption: :traditional)

Also, when reading a password protected zip it doesn't automatically detect the encryption type. It would be nice if @muz and @jphastings can have a look at the AES PR and chime in with their thoughts on how we can support this strategically.

@johnnyshields
Copy link
Contributor Author

Raised PR #211 with docs

@simonoff
Copy link
Member

@johnnyshields I think we can break API because this feature will go into 2.0.0 in any case.

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

Successfully merging this pull request may close these issues.

None yet

4 participants