Refactor: Reusable single-page PDF rendering method in PDF class#8578
Merged
Conversation
… PDF method generatePdfFromCustomCertificate() instantiated mPDF directly to render the single-page certificate with zero margins and mirrorMargins=0, bypassing the PDF class because its constructor hard-codes margin_header/footer=8 and format_pdf() forces mirrorMargins=1 (book layout), which would insert blank pages around the certificate. Move that single-page rendering into a new reusable PDF::singlePageHtmlToPdfDownload() method that keeps the exact same mPDF configuration (zero margins, mirrorMargins=0, A4/A4-L by orientation) and routes remote asset fetches through SafeMpdfHttpClient, so the SSRF guard now lives in one place (pdf.lib.php) instead of being duplicated at each direct mPDF call site. No change to the rendered output. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Instead of instantiating a second mPDF object inside
singlePageHtmlToPdfDownload(), build a PDF instance via the constructor and
reuse its $this->pdf. This leaves a single 'new Mpdf' call (and a single
SafeMpdfHttpClient guard) in the whole class.
To allow the zero-margin certificate layout through the constructor, the
constructor now honors 'margin_header'/'margin_footer' from $params instead of
hard-coding both to 8. Both still default to 8, so callers that don't pass
them are unaffected.
Behavior change: lp_tracking.php builds 'new PDF('A4', 'P', ['margin_footer' => 4, ...])';
that value was previously discarded (footer rendered at 8mm) and now takes
effect (4mm), matching the caller's original intent.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Context
Follow-up to the SSRF fix in #8577 (commit 8468df8). That fix added the
SafeMpdfHttpClientguard to the direct mPDF instantiation insideCertificate::generatePdfFromCustomCertificate(). This PR removes that hand-rolled instantiation and renders the single-page certificate through thePDFclass instead, so the SSRF guard and mPDF construction live in a single place.Why the certificate instantiated mPDF by hand
generatePdfFromCustomCertificate()rendered a single-page certificate with all margins at 0 andmirrorMargins = 0. It could not reuse thePDFclass because the constructor hard-codedmargin_header = 8/margin_footer = 8, andcontent_to_pdf()always callsformat_pdf(), which setsmirrorMargins = 1(book layout) — together these insert blank odd/even pages around a single page.The change
PDF::singlePageHtmlToPdfDownload(string $html, string $fileName, string $orientation): builds aPDFinstance and reuses its$this->pdfmPDF object with all margins at 0 andmirrorMargins = 0, deliberately not callingformat_pdf(). The remote-asset SSRF guard comes from the constructor's existingSafeMpdfHttpClient::container(), so there is now a singlenew Mpdfcall in the whole class.margin_header/margin_footerfrom$paramsinstead of hard-coding both to 8. Both still default to 8 → callers that don't pass them are unaffected.generatePdfFromCustomCertificate()delegates to the new method, keeping the certificate-specific logic (orientation setting, Resource fallback read, file name).SafeMpdfHttpClientimport fromcertificate.lib.php.Behavior change to flag
public/main/my_space/lp_tracking.php:239buildsnew PDF('A4', 'P', ['margin_footer' => 4, 'top' => 40, 'bottom' => 25]). Thatmargin_footer => 4was previously discarded (the constructor hard-coded 8, so the footer rendered at 8 mm); it now takes effect (4 mm), matching the caller's original intent. This is the only existing caller passingmargin_footer, and no caller passesmargin_header. ⚠ The lp_tracking PDF footer spacing changes by 4 mm — worth a visual check during review.Invariant
A single guarded mPDF construction path for the
PDFclass. The certificate render is unchanged (A4/A4-L by orientation, all margins 0,mirrorMargins = 0, download + exit).🤖 Generated with Claude Code