In non-development mode, the CLI shows the user information via the INFO
log level. The message is shown directly without [INFO] as a prefix.
Otherwise it would quickly get annoying to the user seeing [INFO] on
every line of a CLI application.
However, if an error happens it's important for the user to recognize
it's an error or a warning. This commit prints the log level in these
cases.
Now that safe PDFs can open on Windows right after conversion
(implemented in commit 5b2fefd), we need to save/load the "Open safe
documents after converting" setting.
Allowing this would lead to several UI edge-cases related to where the
files would be saved. Avoiding this is the easiest solution at the
moment.
In the future we should consider other options.
It was possible that users would add duplicate documents via 'open with
Dangerzone'. This would lead to unexpected situations and preventing it
both in the CLI and the GUI solves those issues.
Handle the case where a user has already added some documents (either
through 'open with' or via Dangerzone 'select documents' button) and
then they want to add some more via the 'open_with' dialog.
It updates the settings to reflect the newly added documents and blocks
the user from adding them if a conversion is already in progress.
Makes the ContentWidget a choke-point, where we can allow or prevent
adding more documents and where we can ensure that newly selected
documents are added immediately to the DangerzoneGui class.
Logically, the application flow should not change in any way.
Closing windows on macOS would not actually close Dangerzone. Now that
it is a single-window program, it makes sense for it to close
immediately.
Fixes#271
The save group box would get partially trimmed when running in macOS
this appears to be due to differences in rendering fonts and widget
sizes.
Refs #270
If a user has provided an output filename for a document, then we should
no longer accept suffixes. The reason is that we can't do something
meaningful with it, as we can't alter the provided output filename.
The proper behavior is to reject this action with an exception. Note
that this acts more of a safeguard, since (currently) there is no path
where a user may add a suffix to a document that already has an output
filename.
In preparation for adding a limit on how many convert threads exist, we
are simplifying its logic. Getting ocr_lang doesn't seem to belong to
the thread.
Aligns document labels following the design specified in issue #117.
It did not specify how it would change with window resize, so it
currently expands the progress bar / error message width and keeps the
document name fixed in size.
Mypy complains about a line being unreachable. This is probably a false
positive. It must assume the code is not using a framework and thus it
can't when a PySide 'connect()' is being called.
Since everything now happens in a single window, there is no need
to have a way to keep track other windows. They simply won't exist.
But on windows and Linux it will still be possible to start
multiple windows by starting various Dangerzone processes. On MacOS
this doesn't seem to be as easy from the launcher, but it should
not be critical as multiple documents can be converted at the same
time in the one window.
To help debugging and visualizing what was happening, we set all
widgets to be visible at the same time. Now that is no longer needed,
we can hide them.
This keeps the original program flow:
1. select the documents
2. set the settings
3. see the conversion progress
This diverges from the proposed design in issue #117 for simplification
and consistency (with past program flow) purposes.
The user is supposed to only be able to select the safe PDF extension.
In a multi-file scenario, the extension will be the same for all files.
We follow here the design document [1]. To achieve this, we needed a
QLabel right next to a QLineEdit, to give the user the illusion that
it is the same graphical object.
[1]: https://github.com/firstlookmedia/dangerzone/files/6657536/DangerZone_NA02a.pdf
Avoid conversion issues when saving the output file when it is set
wrongly. Inform the user with a red box saying "must end in .pdf"
and prevent the user from clicking "convert" before that is fixed.
Combines the validation logic with the already-existing 'update_ui()'
Set the default directory for saving the file as the one from
the first document. This one will show just the directory name.
If the user changes it by choosing another directory, then show the new
directory name and its full path.
- shows settings again
- removes documents arg from settings widget - this is now stored
under DangerzoneGui instance.
- removes widget 'dangerous_doc_label' - the doc label is already
shown next to each document.
- 'Save as' button now serves the purpose of selecting where all
output files should be saved. Before, it was for selecting where
the file would be saved.
- 'save_lineedit' widget which was read-only and showed the path
where the file would be saved, it now called 'safe_suffix' and is
writable. It is where the user can type the safe file extension
(e.g. '-safe.pdf'). Validation is not yet implemented.
- when 'start_button' is clicked it now changes the output_filename
of all the documents to set their output directory to the one the
user has selected (if 'save_checkbox' enabled) and to set their
new 'safe_suffix'
- change to plural text for selection of multiple documents
These will be needed in for the GUI's settings. This also adds test
cases for these documents. The methods are the following:
- set_output_dir()
For changing the output directory of the safe file
- suffix setter and getter - for changing the suffix of the file
Allows the user to:
a) specify filenames via the terminal (for the GUI)
b) select multiple documents via the GUI
The conversion process can't yet be started since the settings are
broken and disabled (expect mypy complaints).
Allow opening multiple documents at the same time from the terminal
by calling
$ dangerzone document1.pdf document2.pdf
It will open each document in its own window, making use of the
already existing 'multi-document multi-window' parallel conversion
implementation.
plistlib:
- originaly added in commit 3be1d63330
- no longer needed
grp, getpass:
- originally added in commit ae7c919d8e
- used for finding the 'docker' executable. No longer needed.
Checking if files were writeable created files in the process. In the
case where someone adds a list of N files to dangerzone but exits before
converting, they would be left with N 0-byte files for the -safe
version. Now they don't.
Fixes#214
All filename-related exceptions were of class DocumentFilenameException.
This made it difficult to disambiguate them. Specializing them makes it
it easier for tests to detect which exception in particular we want to
verify.
The document's state update is better update in the convert() function.
This is because this function is always called for the conversion
progress regardless of the frontend.
The container output logging logic was in both the CLI and the GUI.
This change moves the core parsing logic to container.py.
Since the code was largely the same, now cli does need to specify
a stdout_callback since all the necessary logging already happens.
The GUI now only adds an stdout_callback to detect if there was an
error during the conversion process.
Initial parallel document conversion: creates a pool of N threads
defined by the setting 'parallel_conversions'. Each thread calls
convert() on a document.
Wildcard arguments like `*` can lead to security vulnerabilities
if files are maliciously named as would-be parameters. In the following
scenario if a file in the current directory was named '--help', running
the following command would show the help.
$ dangerzone-cli *
By checking if parameters also happen to be files, we mitigate this
risk and have a chance to warn the user.
Rename the `gui.common` module and `gui.common.GuiCommon` class
to `gui.logic` and `gui.logic.DangerzoneGui` respectively. We keep as is
the original names of the variables that hold instances of this class,
since they will change in subsequent commits.
This change is part of the initial refactor to make the DangerzoneGui
class handle the GUI logic of the Dangerzone project.
Rename the `global_common` module and `global_common.GlobalCommon` class
to `logic` and `logic.Dangerzone` respectively. Also rename variables
that hold instances of this class.
This change is part of the initial refactor to make the Dangerzone class
handle the core logic of the Dangerzone project.
Let the Document class suggest the default filename for the safe PDF,
based on the provided input filename, appended with the extension
`-safe.pdf`.
Previously, this logic was copy-pasted throughout the code, which made
it difficult to maintain.
Make the Document class always resolve relative input/output file paths,
which are usually passed as arguments by users.
Previously, resolving relative filepaths was a job left to the
instantiators of the Document class. This was error-prone since this
conversion must happen in all the places where we instantiated the
Document class.
Implement Click's callback interface and create validators for the
input/output filenames, using the logic from the Document class. This
way, we can catch user errors as early as possible.
Factor out the filename validation logic and move it into the Document
class. Previously, the filename validation logic was scattered across
the CLI and GUI code.
Also, introduce a new errors.py module whose purpose is to handle
document-related errors, by providing:
* A special exception for them (DocumentFilenameExcpetion)
* A decorator that handles DocumentFilenameException, logs it and the
underlying cause, and exits the program gracefully.
Avoid setting document's filename via document.filename and instead
do it via object instantiation where possible.
Incidentally this has to change some window logic. When
select_document() is called it no longer checks if there is already an
open window with no document selected yet. The user can open as many
windows with unselected documents as they want.
Rename the `common` module and `common.Common` class to `document` and
`document.Document` respectively. Also, rename the variables that hold
instances of this class.
This change reflects the fact that the class is responsible for tracking
the state of the document. When we add bulk document conversion,
allowing us to keep track of a document's state will be key. This name
change is a step towards that.
Container-related methods recently moved to container.py no longer
need to have 'container' in their name as they are within the
container scope already.
Additonally it made it awkward to call from another module:
from .. import container
container.get_container_runtime()
The logic for detecting if we were are running on docker or podman
and identifying its respective binary were scattered across the
codebase. This centralizes it all in container.py
- display_banner() was only displayed in CLI mode so it makes sense
for it to be in the CLI.
- get_version(), was mvoed to util since it is a static function
that is needed in multiple parts of the application.
static methods that are used application-wide should belong to
the utilities python file.
inspired by @gmarmstrong's PR #166 on refactoring global_common
methods to be static and have a dzutil.py
Input_filename and output_filename could be None or Str. This lead
to typing issues where the static analysis type hint tool could not
check that the type colisions would not happen in runtime.
So the logic was replaced by throwing a runtime exception if either
of these valiables is ever used without first having been set.
Originally tied to implment following PEP 589 [1] – TypedDict: Type
Hints for Dictionaries with a Fixed Set of Keys for the Settings
dict.
But this quickly turned out to very challenging without redoing the
code. So we opted instead for using the Any keyword.
[1]: https://peps.python.org/pep-0589/
The following logic was leading to type hint issues:
> inspect.getfile(inspect.currentframe())
But this code is overly complex for what it does is the same as
simply __file__. So we kill two birds with one stone, so to speak.
We added the following check as well:
+ if stdout_callback and p.stdout is not None:
Because, according to the subprocess docs[1]:
> If the stdout argument was not PIPE, this attribute is None.
In this case, it should not need to confirm that p.stdout is not
None in the mypy static analysis. However it still complained. So
we made mypy the favor and confirmed this was the case.
[1]: https://docs.python.org/3/library/subprocess.html#subprocess.Popen.stdout
`subprocess.STARTUPINFO()` only exists in windows systems. Because
of this, in linux-based systems it was raising type hint issues
as it didn't recognize the return function.
There was no code to handle if at this stage the runtime existed.
This caused issues with type hints since `shutil.which()` can
return None, which had not previously been accounted for.
We did not use the opportunity to consolidate all the code for
detecting the runtime, to make this review easier.
The container arguments was duplicated. This could potentially lead
to refactor errors. For example security arg could be added in one
container call but forgotten to be added in a second one.
Moving to /dangerzone was failing with insuficient permissions:
Invalid JSON returned from container: PermissionError: [Errno
13] Permission denied: '/dangerzone/page-3.rgb'
A previous approach was removed in commit 805222. It started with
root at first in a wrapper script and then dropped these
priviledges which running the script.
`--userns=keep-id` solves the mountpoint issues as it maps the user
starting the container is mapped in the container [1].
[1]: https://www.redhat.com/sysadmin/user-flag-rootless-containers
Was calling color spillover to the adjacent text if the banner was
logged instead of printed. Since this is the CLI version, it could
make sense to have this printed.
fixes#144: printing non-ascii characters in a macOS application
opened directly from finder would sometimes lead to an error
message in /var/log/system.log similar to this:
Failed to execute script 'dangerzone' due to unhandled exception:
'ascii' codec can't encode character '\u201c' in position 1:
ordinal not in range(128)
Simplifying the logic for obtaining resource paths by using pathlib
instead inspect.
Co-authored-by: Guthrie McAfee Armstrong <git@gmarmstrong.dev>
Based on commit bbce13d
