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

feat: add support to iterator over encrypted zip #116

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

alandeev
Copy link
Contributor

Adding support to iterate over encrypted archives zip.

ONLY SUPPORTS ZIP ( libarchive limitation )
libarchive documentation -> readme.md

Screenshot from 2023-05-31 09-00-18

Note:
I just implemented it in the iterator, because I don't have time to implement it in the rest, but the idea is the same.

Support for (RAR)
It doesn't support encrypted '.rar' files, because it is a limitation of the libarchive itself, I searched in every way and even after implementing the correction I tried to iterate over a '.rar' file, but I don't get an error and it doesn't display any content either.

the only solution I saw that works is a binding made in unrar.c, but for that I would have to restructure the entire library to implement only this use case for '.rar' file

@alandeev alandeev mentioned this pull request May 31, 2023
Copy link
Member

@otavio otavio left a comment

Choose a reason for hiding this comment

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

Thank you for the pull request! We appreciate your contribution to the project. However, we still have some items to look at before we can merge your code into the repository. I also wanted to let you know that I sent a small code rework to simplify the code. Thank you again for your help!

src/iterator.rs Outdated
@@ -44,6 +44,20 @@ pub enum ArchiveContents {
/// The entry is processed on a return value of `true` and ignored on `false`.
pub type EntryFilterCallbackFn = dyn Fn(&str, &libc::stat) -> bool;

pub struct Password(Vec<u8>);
Copy link
Member

Choose a reason for hiding this comment

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

Needs documentation.

The type would be better named as ArchivePassword?

Copy link
Member

Choose a reason for hiding this comment

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

I renamed, doc is still pending.

src/iterator.rs Outdated
@@ -265,11 +284,11 @@ impl<R: Read + Seek> ArchiveIterator<R> {
/// # Ok(())
/// # }
/// ```
pub fn from_read(source: R) -> Result<ArchiveIterator<R>>
pub fn from_read(source: R, password: Option<Password>) -> Result<ArchiveIterator<R>>
Copy link
Member

Choose a reason for hiding this comment

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

I'd like to have two methods:

  • from_read: same as before
  • from_read_with_password: new

@otavio
Copy link
Member

otavio commented May 31, 2023

Also, CI is failing but it works locally; any idea?

@alandeev
Copy link
Contributor Author

alandeev commented Jun 1, 2023

I didn't understand either, I realized that when I run the "cargo test" it breaks in my test, but if I run my test individually it works, it's like I'm mocking some function I'm using..

And regarding the fixes, I'll try to do it tomorrow, all the points you commented make total sense.

@otavio
Copy link
Member

otavio commented Jun 1, 2023

If it fails in your machine, it might be a race of memory corruption. Check in cargo-valgrind.

@otavio
Copy link
Member

otavio commented Jun 1, 2023

@hivexdev progress. The failing tests are fixed. The Windows failure is unrelated.

@otavio
Copy link
Member

otavio commented Jun 26, 2023

@hivexdev have you looked at this?

@AngelineNinu
Copy link

can the passphrase be non ascii , will this work for decrypting a password protectd zip?

@AngelineNinu
Copy link

can non ascii password be used to decrypt a paswd protectd zip file

@coveralls
Copy link

Pull Request Test Coverage Report for Build 5148429169

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

Details

  • 3 of 3 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+1.1%) to 85.049%

Totals Coverage Status
Change from base Build 5093406152: 1.1%
Covered Lines: 347
Relevant Lines: 408

💛 - Coveralls

@coveralls
Copy link

coveralls commented Mar 14, 2024

Pull Request Test Coverage Report for Build 5148403204

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

Details

  • 3 of 3 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.1%) to 84.069%

Totals Coverage Status
Change from base Build 5093406152: 0.1%
Covered Lines: 343
Relevant Lines: 408

💛 - Coveralls

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

Successfully merging this pull request may close these issues.

4 participants