Skip to content

fix: file I/O safety for UFO save and binary import#117

Merged
kostyafarber merged 7 commits into
mainfrom
fix/file-io-safety
Jul 2, 2026
Merged

fix: file I/O safety for UFO save and binary import#117
kostyafarber merged 7 commits into
mainfrom
fix/file-io-safety

Conversation

@kostyafarber

Copy link
Copy Markdown
Collaborator

Fixes the file-safety blockers from the 2026-07 codebase review, plus follow-ups from adversarial review of the first pass.

User-file safety

  • Malformed binary fonts error instead of panicking — all unwraps in the binary reader (outlines, draw, hmtx) converted to FormatBackendError::Binary with glyph/table context; hmtx lookup hoisted out of the per-glyph loop.
  • Fractional coordinates preserved in UFO saveUfoRound deleted; points, component offsets, anchors, guidelines, and advances write raw f64. fontc export is unaffected (it rounds at glyf generation). Exact round-trip test added.
  • Invalid names fail loudly — all ten Name::new().unwrap() sites replaced with FormatBackendError::UfoName naming the offending glyph/anchor/group. No silent sanitizing.
  • Atomic UFO save — the writer previously remove_dir_all'd the destination before a non-atomic write; any failure destroyed the original. Now: stage the complete UFO in a temp sibling dir, swap, restore on failure (with the preserved path in the error if even the restore fails).
  • Bonus bug found during the rewrite: features.fea was written before norad's save, which internally deletes the destination — feature source was silently destroyed on every UFO save. Fixed (norad serializes features itself) with a round-trip test.

Durability follow-ups

  • Directory fsync cfg-gated for Windows — in both shift-backends and shift-source. File::open(dir) fails on Windows, so every UFO and .shift save would have errored after succeeding. Unix-only now; NTFS journals metadata.
  • Staged UFO tree fully fsynced before the swap (files first, directories bottom-up) so a post-save power loss can't leave durable renames with non-durable glif contents.
  • Atomic rename-exchange for the swaprenamex_np(RENAME_SWAP) on macOS, renameat2(RENAME_EXCHANGE) on Linux, eliminating the crash window where no UFO existed at the target path. Runtime fallback to the two-rename dance on filesystems without swap support (ENOTSUP/EINVAL/ENOSYS) and on other platforms; the fallback path keeps direct test coverage.

Verification

  • cargo test --workspace: 299 passed, 0 failed (+11 new tests: fractional round-trip, malformed/truncated fonts, failed-save-preserves-original, invalid names, feature round-trip, nested-layer fsync walk, exchange + fallback swap).
  • fmt clean, clippy zero warnings across the workspace.

Follow-ups tracked separately: versioned backup retention (#116), designspace multi-UFO writer and interop safety (upcoming).

🤖 Generated with Claude Code

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant