fix: Round 8 polish — encrypt wipe, update worker leak, CLI, DPR change#79
Merged
Conversation
QLineEdit text for owner/user/decrypt passwords persisted in memory for the entire session. Now cleared via try/finally in _run() to align with the broader wipe_pdf_password() pattern. Addresses R8-H1.
The _update_worker QObject lived for the lifetime of the application because only _update_thread.deleteLater was wired. Now released in _on_update_found() via _release_update_worker() and called defensively from closeEvent. Also removes the dead _update_ready signal: it was declared at the class level and connected to _notify_update, but never emitted — the actual update notification path is via _update_worker.done. Addresses R8-H2 and R8 dead-code finding.
Previously only sys.argv[1] was opened, so multi-file "Open With" or drag-and-drop of N>1 PDFs onto the executable silently dropped every file after the first. -h / --help was treated as an invalid path and the app launched into the welcome screen. Now uses argparse: accepts multiple PDF paths (each opens in its own tab via _load_and_track), and exposes standard --help / --version flags that exit before QApplication is constructed. Addresses R8-M1.
Both viewer/canvas.py and editor/canvas.py read devicePixelRatioF() only inside _schedule_visible(). Without a screenChanged handler, moving the window between 100% and 200% monitors (or changing the DPR of an active monitor via Windows Display Settings) left every visible page blurry until the next zoom interaction. New showEvent override connects to window.windowHandle().screenChanged and invalidates cached pixmaps on DPR change, then re-queues the visible range. The connect is guarded against duplicate handlers on re-show / tab switch. Addresses R8/D1.
PdfReader(wm_path) raised PdfReadError cryptically when the watermark PDF was itself encrypted (rare but observed with corporate stamp PDFs). Now a dedicated _prompt_watermark_password() helper prompts for the watermark's password before pre-flight and the worker decrypts the second reader with it, kept separate from self._pdf_password so source and watermark credentials don't mix. Addresses R8 bonus #6.
A malformed outline (cyclic refs, bad page indexes, unexpected entry shape) could raise mid-build and leave the TOC panel in an inconsistent state — half-populated and surfaced as a stack trace. Now the failure is logged via logging.warning and the TOC tree is reset to an empty / hidden state so the rest of the viewer stays usable. Addresses R8 bonus #7.
10 new tests guard the PR-C wiring against silent regressions: - encrypt._clear_password_fields() + try/finally hookup - _release_update_worker helper + _update_ready signal removal - argparse CLI (multi-PDF, --help, --version) — also exercises the subprocess version/help flags end-to-end - screenChanged DPR handler on viewer + editor canvases - watermark encrypted PDF prompt path - _populate_toc try/except + logging
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| import sys | ||
| from pathlib import Path | ||
|
|
||
| import pytest |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes 7 selective findings from audit Round 8: 2 HIGH, 3 MEDIUM, 2 BONUS polish items.
Fixes
app/tools/encrypt.py) — QLineEdits persisted password text. New_clear_password_fieldsinvoked from try/finally in_run().app/window.py) —_update_workerQObject neverdeleteLater()'d. New_release_update_workerhelper.pdfapps.py) —-h/--help/--versionnow work; accepts multiple PDF paths vianargs="*".app/viewer/canvas.py,app/editor/canvas.py) — canvases now react toQWindow.screenChangedand invalidate cached pixmaps._update_readysignal removed (app/window.py).Bonus fixes
app/tools/watermark.py) —_prompt_watermark_passwordhandles encrypted watermarks gracefully._populate_tocgraceful failure (app/viewer/panel.py) — wrapped in try/except +logging.warning.Tests
10 new tests in
tests/test_round8_fixes.pycovering encrypt wipe, update worker release, CLI argparse, DPR handlers, watermark prompt, and TOC failure.Final
python pdfapps.py --versionand--helpworkSupersedes #77.