SebastianSimon / firefox-omni-tweaks

A script that disables the clickSelectsAll behavior of Firefox, and more.
MIT License
43 stars 8 forks source link

Lack of unzip errors is not handled #6

Closed Botster closed 4 years ago

Botster commented 4 years ago

On my system (openSUSE Leap 15.2) using UnZip 6.00 of 20 April 2009, by Info-ZIP (Maintained by C. Spieler.), there are no errors produced when unzipping "${firefox_dir}/browser/omni.ja" at line 53. The condition of an empty unzip_errors variable is not handled, and this causes the script to fail for a non-existent error condition.

I fixed that by modifying line 56 to: if [[ -z "$unzip_errors" ]] || (echo "${unzip_errors}" | xargs | grep --extended-regexp --quiet "${expected_errors}"); then

I hope this helps.

SebastianSimon commented 4 years ago

Dang, I knew this was too unsafe…

Does the unzip command from the script have a 0 status code for you? That could be an alternative check. For reference, it has status code 2 for me which, according to the man page, means:

A generic error in the zipfile format was detected. Processing may have completed successfully anyway; some broken zipfiles created by other archivers have simple work-arounds.

When I unzip the omni.ja after zipping it, I don’t get errors. However, does it never produce errors when unzipping the omni.ja for you, not even when first unzipping?

Is the output truly empty or is it e.g. a newline or other whitespace?

SebastianSimon commented 4 years ago

Actually, a much better check may be this: if after the unzip command, the unzipped files exist within the /tmp/omni directory, then proceed normally. If no error or the expected warning and error are seen, don’t report them. If an unexpected error is seen, report it. If the files are not found, abort.

SebastianSimon commented 4 years ago

Can you confirm if commit 3212e37 fixes this issue?

Botster commented 4 years ago

While trying to confirm, I got an error: "Error: You need to start and close Firefox again to apply the changes before running this script." Apparently Firefox did not automatically remove the /usr/lib64/firefox/browser/.purgecaches file. I manually removed it.

Since omni.ja is the same size as it was 8 hours ago, I cannot confirm that the script actually did anything. However, it completed successfully with no errors. And, a click on my Firefox browser's location bar behaves as it should.

Thank you for the work you have put in to this.

SebastianSimon commented 4 years ago

"Error: You need to start and close Firefox again to apply the changes before running this script." Apparently Firefox did not automatically remove the /usr/lib64/firefox/browser/.purgecaches file. I manually removed it.

This error tells you to open and close Firefox again. This is unrelated to the unzipping issue. A clean test would be to execute this script with the original, unmodified omni.ja file, after restarting and closing Firefox, so that any such test is reproducible. If you already executed the script, then the omni.ja is already modified — the script shouldn’t be executed after that (the sed modifications silently fail, then). So either restore the backup or wait for the next Firefox upgrade to start out with an unmodified omni.ja.

I’ve updated the README to explain the .purgecaches mechanism in detail.