fix: preserve existing mimeapps.list entries when installing web url handler

[crowecawcaw]

Fixes:

What was the problem/requirement? (What/Why)

deadline handle-web-url --install opened the user's mimeapps.list in "w" mode, which truncates the file. Installing the Deadline URL handler therefore wiped every unrelated default-application association (browser, PDF viewer, mailto handler, etc.) — permanent data loss for the user.

What was the solution? (How)

Read and parse the existing mimeapps.list (an INI-style file) with configparser, add/update only the x-scheme-handler/deadline entry in [Default Applications], and write the file back preserving all other entries. The missing-file case is handled (a fresh file with just the deadline entry). configparser is created with interpolation=None and optionxform=str so mime-type/scheme keys keep their case and % characters don't break parsing; entries are written in the key=value.desktop; freedesktop format.

What is the impact of this change?

--install no longer destroys existing default-app associations. The deadline-owned .desktop file write path is unchanged.

How was this change tested?

Added a red-green regression test that pre-populates mimeapps.list with unrelated associations, runs the install path, and asserts the unrelated entries survive while the deadline handler is added, plus a test for the missing-file case.

• Have you run the unit tests? Yes — test/unit/deadline_client/cli/test_cli_handle_web_url.py (73 passed, 2 platform-gated skips).
• Have you run the integration tests? No (pure client-side file-handling logic).

Was this change documented?

• Are relevant docstrings in the code base updated? N/A — no signature/behavior contract change.
• Has the README.md been updated? No — no CLI argument changes.

Does this PR introduce new dependencies?

• This PR adds one or more new dependency Python packages.
• This PR does not add any new dependencies. (configparser is stdlib.)

Is this a breaking change?

No.

Does this change impact security?

No new threat surface; it prevents destruction of user-owned files (data-loss fix).

Testing

Automated

Unit tests in test/unit/deadline_client/cli/test_cli_handle_web_url.py cover every changed behavior and its failure mode (78 passed, 2 platform-gated skips on this run):

• test_linux_install_preserves_existing_mimeapps_entries — pre-existing associations survive; deadline handler added
• test_linux_install_creates_mimeapps_when_missing — fresh file created with the handler entry
• test_linux_install_raises_on_malformed_mimeapps — non-INI file surfaces as DeadlineOperationError, not a raw traceback
• test_linux_install_tolerates_duplicate_mimeapps_entries — duplicate keys/sections tolerated (strict=False, last value wins)
• test_linux_install_preserves_mimeapps_when_write_fails — atomic write: on mid-write failure the original file is untouched and no temp files are left behind
• test_linux_install_preserves_mimeapps_permissions / test_linux_install_creates_mimeapps_with_default_permissions — existing file mode preserved across the temp-file rewrite; new file gets 0644 (not mkstemp's 0600)

Manual end-to-end verification

Ran the real install_deadline_web_url_handler() code path in an isolated throwaway $HOME (never touching the real ~/.config), with a stub deadline/update-desktop-database on PATH so shutil.which and subprocess.run executed for real. Host was macOS with sys.platform patched to linux; the filesystem behavior (configparser round-trip, mkstemp, chmod, os.replace) is identical, but this has not been exercised on a physical Linux desktop session.

Before (~/.config/mimeapps.list, deliberately chmod'ed to 0640):

-rw-r----- (0o640)
# my precious comment
[Default Applications]
text/html=firefox.desktop
x-scheme-handler/http=firefox.desktop
x-scheme-handler/mailto=thunderbird.desktop
application/pdf=okular.desktop

[Added Associations]
image/png=gimp.desktop;eog.desktop;

After install:

-rw-r----- (0o640)
[Default Applications]
text/html=firefox.desktop
x-scheme-handler/http=firefox.desktop
x-scheme-handler/mailto=thunderbird.desktop
application/pdf=okular.desktop
x-scheme-handler/deadline=deadline.desktop;

[Added Associations]
image/png=gimp.desktop;eog.desktop;

Observed results (19/19 checks passed):

• All four pre-existing [Default Applications] associations and the [Added Associations] section/entry survived; the deadline handler was appended.
• File permissions preserved (0640 before and after); a freshly created file gets 0644.
• Re-running the install is idempotent (byte-identical file).
• Absent file: created correctly with just the handler entry.
• Malformed file (key before any section header): raises DeadlineOperationError with a clear message and leaves the original file byte-for-byte untouched.
• Duplicate keys and duplicate sections: install succeeds, last value wins, other entries preserved.
• No .mimeapps.list.*.tmp files left behind in ~/.config.
• Known trade-off (flagged in review): comments (# ...) and blank-line formatting are not preserved by the configparser round-trip — associations are, which is the data this fix protects.

Note: this verification ran on macOS with the platform patched; a smoke test on a real Linux desktop (checking the DE actually resolves deadline:// URLs after install and that existing browser/PDF defaults still work) would be a worthwhile final check.

[github-actions]

configparser.read() on an existing mimeapps.list can raise on any content that is not strictly valid INI — e.g. MissingSectionHeaderError (stray key before a section), DuplicateOptionError/DuplicateSectionError (default strict=True), or a UnicodeDecodeError. That exception now propagates uncaught out of install_deadline_web_url_handler, crashing the CLI with a raw traceback.

This is a behavior change from the old truncate-and-write path, which tolerated any prior content. Every other failure mode in this function is surfaced as a DeadlineOperationError; consider wrapping the read() in try/except and raising DeadlineOperationError for consistency (and to avoid an install failing on a slightly-malformed but otherwise usable mimeapps.list).

[crowecawcaw]

Addressed in 0dc7a86: the read() is now wrapped in try/except (configparser.Error, UnicodeDecodeError) and surfaced as a DeadlineOperationError, consistent with the other failure modes in this function. Covered by test_linux_install_raises_on_malformed_mimeapps.

[github-actions]

Note: configparser round-tripping preserves all associations (the important data this fix targets), but it does not preserve comments (# ... lines) or blank-line formatting in the original mimeapps.list. Those are silently dropped on rewrite. This is almost certainly acceptable in practice — comments in mimeapps.list are rare — but it does mean the "preserve everything" framing in the comment/PR isn't strictly complete. No change required if that trade-off is intended; flagging it since the change is explicitly about avoiding data loss.

[crowecawcaw]

Acknowledged — dropping comments/blank lines on rewrite is an intended trade-off. The goal of this change is preserving the association data; comments in mimeapps.list are rare and other desktop tools (e.g. GLib's key-file writer) drop them too. Keeping configparser avoids hand-rolling an INI round-tripper for this edge case.

[github-actions]

Consider passing strict=False to ConfigParser(...). With the default strict=True, configparser raises DuplicateOptionError/DuplicateSectionError on any repeated key or section — and real-world mimeapps.list files written by other desktop tools have historically contained duplicate keys (especially in [Added Associations]). On such a file, the current code converts what used to be an "install always succeeds" path into a hard DeadlineOperationError, so the handler can't be installed at all until the user hand-edits their file.

Since this change is specifically about being robust to whatever is already in mimeapps.list, strict=False (last-value-wins) would tolerate these files while still preserving unrelated associations, which seems closer to the intent. Genuinely malformed files (no section header, bad encoding) would still be caught by the surrounding try/except.

[crowecawcaw]

Addressed in dbade12: ConfigParser is now constructed with strict=False so duplicate keys/sections in an existing mimeapps.list are tolerated (last value wins). Covered by test_linux_install_tolerates_duplicate_mimeapps_entries.

[github-actions]

This rewrite is not atomic: open(mimeapps_list_file_path, "w") truncates the existing file before mimeapps.write() runs. If the process is interrupted between truncation and a successful write (crash, kill, disk-full, or an exception raised inside write()), the file is left empty or partially written — losing precisely the unrelated default-application associations this change was written to protect.

Consider writing to a temp file in the same directory and os.replace()-ing it into place so the original is only replaced once the new content is fully and successfully written:

dir_name = os.path.dirname(mimeapps_list_file_path)
fd, tmp_path = tempfile.mkstemp(dir=dir_name, prefix=".mimeapps.list.", suffix=".tmp")
try:
    with os.fdopen(fd, "w") as tmp:
        mimeapps.write(tmp, space_around_delimiters=False)
    os.replace(tmp_path, mimeapps_list_file_path)
except BaseException:
    os.unlink(tmp_path)
    raise

[crowecawcaw]

Addressed in 58febaa: the rewrite now renders to a mkstemp temp file in the same directory and os.replace()s it into place, with the temp file unlinked on failure. Covered by test_linux_install_preserves_mimeapps_when_write_fails.

[github-actions]

The atomic-write here does not preserve the destination file's permissions. tempfile.mkstemp() creates the temp file with mode 0600 (owner read/write only), and os.replace() keeps that mode on the final mimeapps.list.

For the all_users case (/usr/share/applications/mimeapps.list, written as root) this is a functional regression: the system-wide file needs to be world-readable so every user's desktop environment can resolve associations, but after this change it becomes root-only (0600) — and it silently drops the permissions on a pre-existing file that was previously e.g. 0644.

Consider stat-ing the existing file first (if present) and applying its mode to the temp file before os.replace(), or applying a sane default honoring umask (e.g. 0644) when the file is new. mkstemp's hardcoded 0600 is rarely what you want for a shared config file.

[crowecawcaw]

Addressed in 08ec80f: the existing file's mode is stat()ed before the rewrite and applied to the temp file via os.chmod() before os.replace(); a newly created mimeapps.list defaults to 0644. Covered by test_linux_install_preserves_mimeapps_permissions and test_linux_install_creates_mimeapps_with_default_permissions.