Skip to content

Fix SS export when image names (without extension) conflict (BL-16498)#8042

Open
StephenMcConnel wants to merge 3 commits into
Version6.4from
BL-16498-ErrorInSSExport
Open

Fix SS export when image names (without extension) conflict (BL-16498)#8042
StephenMcConnel wants to merge 3 commits into
Version6.4from
BL-16498-ErrorInSSExport

Conversation

@StephenMcConnel

@StephenMcConnel StephenMcConnel commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Fixes the error thrown during Spreadsheet (SS) export when a book contains image files whose names, stripped of extension, collide.

Problem

EPPlus treats worksheet drawing names as case-insensitive and rejects duplicates. Two source files such as image.jpg and image.png (or Foo.png and foo.jpg) reduce to the same drawing name, so worksheet.Drawings.AddPicture(...) threw during export.

Fix

  • d34355b92 — make the duplicate-name check case-insensitive and rename a colliding image to {name}-{rowNum}.
  • 74e34419d — guarantee the renamed value is actually unique. The first commit appended the -{rowNum} suffix blindly (no re-check) and renamed using the original casing, so the result could still collide (e.g. three images sharing a base name in one row, or two same-row names differing only in case). Now we loop, comparing case-insensitively, until the name is truly unique — matching EPPlus semantics.

Testing

  • Full Spreadsheet test suite passes (376 tests).
  • BloomExe and BloomTests build cleanly.

Ref: https://issues.bloomlibrary.org/issue/BL-16498

Devin review


This change is Reviewable

StephenMcConnel and others added 2 commits July 7, 2026 12:09
The initial fix renamed a colliding image to `{name}-{rowNum}` but appended
the suffix blindly without re-checking, and compared case-insensitively while
renaming with the original casing. So the renamed value could still collide
(e.g. three images sharing a base name in one row, or two same-row names
differing only in case), causing AddPicture to throw the very EPPlus
name-collision exception BL-16498 set out to prevent. Loop until the name is
truly unique (case-insensitively), matching EPPlus semantics.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@greptile-apps

greptile-apps Bot commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR fixes a crash during spreadsheet export when a book contains image files whose base names (after stripping the extension) collide — including case-insensitive collisions such as conflict.png / Conflict.jpg — by replacing a broken single-attempt, case-sensitive deduplication check with a case-insensitive do...while loop that keeps incrementing the name suffix until EPPlus's uniqueness constraint is satisfied.

  • SpreadsheetIO.cs: The embedImage local function now defines a NameIsTaken helper that compares candidate drawing names case-insensitively against all existing worksheet drawings, then loops (do...while) with an incrementing suffix until a genuinely unique name is found — matching EPPlus's own case-insensitive uniqueness semantics.
  • SpreadsheetImagesTests.cs: Adds a regression test (Export_ImagesWithConflictingNames_AllEmbeddedWithoutError) that creates three images whose base names collide both by extension and by case, exports the book, re-reads the .xlsx file, and asserts all three images are embedded without error or warning.

Important Files Changed

Filename Overview
src/BloomExe/Spreadsheet/SpreadsheetIO.cs Replaces a case-sensitive, single-attempt duplicate-name check with a case-insensitive do...while loop that keeps incrementing the suffix until the drawing name is genuinely unique, matching EPPlus's own case-insensitive uniqueness requirement.
src/BloomTests/Spreadsheet/SpreadsheetImagesTests.cs Adds a regression test that exports a book with three images whose base names collide both exactly and case-insensitively, then verifies all three embed without error and no warnings are raised.

Reviews (2): Last reviewed commit: "Add regression test for SS export image-..." | Re-trigger Greptile

@StephenMcConnel

Copy link
Copy Markdown
Contributor Author

[Claude Opus 4.8 (1M context)] Consulted Devin on 2026-07-07 20:13 UTC up to commit 74e34419d — 0 Bugs, 0 Flags (clean). Greptile also reviewed clean.

Exports a small book whose three image files reduce to the same EPPlus
drawing name once the extension is dropped — including names that differ
only in case (conflict.png / Conflict.jpg / conflict.gif) — and asserts
every image embeds with no error message and no warning.

Verified the test fails against the pre-fix code (the case-differing image
was reported as a "Bad image file" because EPPlus rejected the duplicate
drawing name) and passes with the fix.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@StephenMcConnel

Copy link
Copy Markdown
Contributor Author

[Claude Opus 4.8 (1M context)] Consulted Devin on 2026-07-07 20:54 UTC up to commit e3c3d371e — 0 Bugs, 0 Flags. (3 Informational observations only; not blocking.) Greptile also re-reviewed clean.

@StephenMcConnel StephenMcConnel marked this pull request as ready for review July 7, 2026 21:13
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.

2 participants