Skip to content

Taking a diff hunk on a binary file (png, ico, dll) rewrites it as mangled UTF-8 and corrupts it #467

Description

@matt-edmondson

What's wrong

The similar-repo comparison diffs every tracked path that two repositories share as text, and never checks whether the file is binary.

  • DiffRepos (ProjectDirector/ProjectDirector.cs ~L1392-1397) reads each matched file through ReadFileOrEmpty, which calls File.ReadAllText (~L1411), and then runs Differ.Instance.CreateLineDiffs on the result.
  • ShowComparedRepo (~L1544-1548) lists every file whose diff has changed lines. A binary that differs between the two repos, such as icon.png or a vendored .dll, is listed with nonsense line counts, and clicking it opens the side-by-side view with hunk arrows.
  • The hunk arrows in ShowDiffLeft and ShowDiffRight rebuild the file from diff.PiecesOld/diff.PiecesNew, joined with Environment.NewLine, and write it with File.WriteAllText (~L1651, ~L1746).

File.ReadAllText decodes as UTF-8, so every invalid byte sequence becomes U+FFFD, and File.WriteAllText writes it back as EF BF BD. Joining with Environment.NewLine also rewrites every 0x0A/0x0D byte. What gets written is neither the source binary, nor the destination binary, nor the selected hunk. It is corrupted, including the "unchanged" bytes around the hunk.

Failure scenario

  1. Two sibling repositories both track icon.png, and the images differ.
  2. Select repo A, open the comparison with repo B, and click icon.png in "Similar Files".
  3. Click any hunk arrow.
  4. icon.png in the target repo is overwritten with mangled bytes and no longer opens as an image. Nothing warns the user, and the app's Commit button stages it with add --all.

Large binaries, such as LFS assets, are also fully read and line-diffed against every sibling on each selection, which is wasted work.

Suggested fix

  • In DiffRepos, classify each matched path as binary before diffing. Git's heuristic is a NUL byte in the first 8000 bytes; git check-attr / git diff --no-index --numstat also work. Compare a binary path by bytes or hash (identical vs differs), and don't build a line diff for it.
  • In ShowComparedRepo, mark binary entries and offer no per-hunk arrows for them. Copying the whole file is the only sensible action for a binary.
  • Optionally compare files above a size cap by hash instead of line-diffing them.

Acceptance criteria

  • A differing binary tracked by both repos never shows hunk-take arrows.
  • No code path calls File.WriteAllText on a file detected as binary.
  • A test runs the diff on two repos whose shared .png differs and asserts that no line diff with hunks is produced for it.

Activity

  1. matt-edmondson commented on Oct 6, 2026

    @matt-edmondson
    ContributorAuthor

    Triage

    • Category: Bug
    • Priority: High. Clicking a hunk arrow on a binary silently corrupts the file in the target repo, and the app's Commit button then stages the corrupted file with add --all. Nothing warns the user, and the damage is easy to commit before anyone notices.
    • Area / suggested owner: Similar-repo comparison and diff view (DiffRepos, ShowComparedRepo, and ShowDiffLeft/ShowDiffRight in ProjectDirector/ProjectDirector.cs).
    • Duplicates / in progress: Not a duplicate. It is closely related to Taking one diff hunk rewrites every line ending in the file and strips its UTF-8 BOM #445, where taking a hunk rewrites line endings and strips the BOM, because both come from the same text round-trip in the hunk-write path. Fixing them together would be sensible. No open PR covers it.

    Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions