dtrx-py / dtrx

Do The Right Extraction
GNU General Public License v3.0
224 stars 10 forks source link

Bail out when dealing with password-protected archives in --nonintera… #17

Closed dilinger closed 2 years ago

dilinger commented 2 years ago

…ctive mode

Archives that contain a password will pause program execution and wait for the user to input the password. This is normally fine and good, but when the user specifies --noninteractive, they definitely don't want to be prompted.

Instead of ignoring the noninteractive option for password archives, kill the prompting process and throw an ExtractorError. Because the extractor probably left the terminal in non-echo mode, we also need to run 'stty echo' to turn character echo back on. This is probably not portable, so if dtrx is being used on non-linux/bsd systems we should figure out a better solution.

This patch should fix issue #14.

Normally with a password archive, we spit out a lot of debugging info in an error scenario. However, since we know exactly what went wrong here, we nuke all that stuff from stderr in order to have more readable output:

dilinger@e7470:/home/dev/dilinger-dtrx$ ./scripts/dtrx -n file.zip

[/home/dev/dilinger-dtrx/file.zip] test2 password: Enter password (will not be echoed):dtrx: ERROR: could not handle file.zip ERROR:dtrx-log:could not handle file.zip dtrx: ERROR: treating as Zip file failed: cannot extract '/home/dev/dilinger-dtrx/file.zip' in non-interactive mode ERROR:dtrx-log:treating as Zip file failed: cannot extract '/home/dev/dilinger-dtrx/file.zip' in non-interactive mode dtrx: ERROR: treating as 7z file failed: cannot extract '/home/dev/dilinger-dtrx/file.zip' in non-interactive mode ERROR:dtrx-log:treating as 7z file failed: cannot extract '/home/dev/dilinger-dtrx/file.zip' in non-interactive mode dilinger@e7470:/home/dev/dilinger-dtrx$

erhan- commented 2 years ago

Great job @dilinger . One idea: "cannot extract encrypted archive '%s' in non-interactive mode"

erhan- commented 2 years ago

Any encrypted zip will do anyway.

dilinger commented 2 years ago

I can try my hand at adding a few tests. I'm thinking a basic encrypted archive test (if I can figure out how to pipe a password to it), and then a non-interactive test that verifies a failure on the password archive. I'm not sure that it's worth testing multiple types of password archives, or nested archives, but I could be convinced otherwise. Are there any other tests that should be added?

noahp commented 2 years ago

That sounds perfect :+1: ... most of the existing tests are single happy-path only (except for the ones catching specific error paths). At some point it might be worth investing in pytest but I haven't been motivated yet :sweat_smile:

dilinger commented 2 years ago

Actually, I can't get it to work and am giving up. I can't figure out how to pipe a password to unzip, and the following just hangs when I run compare.py:

I also had a grep in there for an error message, but it doesn't make a difference; it just hangs. The archive is created with 'echo test > test; zip -P foobar encrypted.zip test'

dilinger commented 2 years ago

I did modify the error message to be "cannot extract encrypted archive '%s' in non-interactive mode", though. Let me know if I should push that.

noahp commented 2 years ago

I added a single happy path test here: https://github.com/dilinger/dtrx/pull/1

feel free to squash into your branch!

dilinger commented 2 years ago

The only difference in that first commit is the error message string.

erhan- commented 2 years ago

Thank you very much for your contribution. I think this was the last thing that stopped non-interactive batch processing for me.

noahp commented 2 years ago

I'm terribly sorry, I neglected to merge this! @dilinger is this ready to ship?

dilinger commented 2 years ago

Yes, should be good to go. Thanks!