Skip to content

Fix duplicate HTML content in HtmlMonographFilePlugin#2310

Open
zielaq wants to merge 1 commit intopkp:stable-3_5_0from
zielaq:fix/html-monograph-duplicate-content
Open

Fix duplicate HTML content in HtmlMonographFilePlugin#2310
zielaq wants to merge 1 commit intopkp:stable-3_5_0from
zielaq:fix/html-monograph-duplicate-content

Conversation

@zielaq
Copy link
Copy Markdown

@zielaq zielaq commented Apr 15, 2026

Summary

  • Fix TypeError in HtmlMonographFilePlugin::downloadCallback() when viewing HTML publication format files not assigned to a chapter
  • $submissionFile->getData('chapterId') returns null for non-chapter files, causing ChapterDAO::getChapter(int) to throw a TypeError in PHP 8
  • Hook::run() silently catches the plugin exception and continues, so the hook returns CONTINUE instead of ABORTCatalogBookHandler::download() then sends the file a second time via app()->get('file')->download(), resulting in duplicate HTML content

Fix

Guard the getChapter() call with a null check (same pattern already used in CatalogBookHandler::download() at line 420):

// Before
$chapter = $chapterDao->getChapter($submissionFile->getData('chapterId'));

// After
$chapterId = $submissionFile->getData('chapterId');
$chapter = $chapterId ? $chapterDao->getChapter((int) $chapterId) : null;

Test plan

  • Upload an HTML file as a publication format without assigning it to a chapter
  • Publish the submission and view the HTML file via /catalog/view/{id}/{formatId}/{fileId}
  • Verify the HTML content appears only once (not duplicated)
  • Upload an HTML file assigned to a chapter and verify it still works correctly

@CLAassistant
Copy link
Copy Markdown

CLAassistant commented Apr 15, 2026

CLA assistant check
All committers have signed the CLA.

When an HTML publication format file is not assigned to a chapter,
$submissionFile->getData('chapterId') returns null. Passing null to
ChapterDAO::getChapter(int $chapterId) causes a TypeError in PHP 8.

The Hook::run() exception handler silently catches plugin exceptions
and continues, so downloadCallback fails to return Hook::ABORT.
CatalogBookHandler::download() then falls through to
app()->get('file')->download(), sending the file content a second time.

This fix guards the getChapter() call with a null check, consistent
with the existing pattern in CatalogBookHandler::download() (line 420).
@zielaq zielaq force-pushed the fix/html-monograph-duplicate-content branch from 36417e3 to 843a190 Compare April 15, 2026 17:36
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