-
Notifications
You must be signed in to change notification settings - Fork 14
fix: prevent PDF export crash on tables with nested quotes in inline styles #86
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -468,30 +468,89 @@ private function cleanTableHtml($html) | |||||||||||||
| // Remove colgroup entirely (causes fixed widths) | ||||||||||||||
| $html = preg_replace('/<colgroup\b[^>]*>.*?<\/colgroup>/is', '', $html); | ||||||||||||||
|
|
||||||||||||||
| // Remove table-layout:fixed style (prevents auto-sizing) | ||||||||||||||
| $html = preg_replace('/table-layout\s*:\s*fixed\s*;?/i', '', $html); | ||||||||||||||
| // Parse with DOMDocument rather than regexes: style/attribute values coming | ||||||||||||||
| // from pasted web content can contain nested quotes (e.g. style="...url('...')..."), | ||||||||||||||
| // which regex-based quote matching cannot handle reliably and ends up corrupting | ||||||||||||||
| // the markup fed to TCPDF (causing crashes on malformed HTML). | ||||||||||||||
| $dom = new DOMDocument(); | ||||||||||||||
| libxml_use_internal_errors(true); | ||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
and after |
||||||||||||||
| $dom->loadHTML( | ||||||||||||||
| '<?xml encoding="utf-8" ?><div>' . $html . '</div>', | ||||||||||||||
| LIBXML_HTML_NOIMPLIED | LIBXML_HTML_NODEFDTD, | ||||||||||||||
| ); | ||||||||||||||
| libxml_clear_errors(); | ||||||||||||||
|
|
||||||||||||||
| $wrapper = $dom->getElementsByTagName('div')->item(0); | ||||||||||||||
| if ($wrapper === null) { | ||||||||||||||
| // Fallback: parsing failed unexpectedly, keep original content rather than losing it | ||||||||||||||
| return $html; | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| foreach (iterator_to_array($dom->getElementsByTagName('table')) as $table) { | ||||||||||||||
| // Remove width/height (attributes and styles) on the table and its rows/cells | ||||||||||||||
| $xpath = new DOMXPath($dom); | ||||||||||||||
|
Comment on lines
+489
to
+491
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||
| foreach ($xpath->query('.//td | .//th | .//tr | .', $table) as $node) { | ||||||||||||||
| if (!($node instanceof DOMElement)) { | ||||||||||||||
| continue; | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| $node->removeAttribute('width'); | ||||||||||||||
| $node->removeAttribute('height'); | ||||||||||||||
|
|
||||||||||||||
| $style = $this->removeStyleProperties($node->getAttribute('style'), ['width', 'height', 'table-layout']); | ||||||||||||||
| if ($style === '') { | ||||||||||||||
| $node->removeAttribute('style'); | ||||||||||||||
| } else { | ||||||||||||||
| $node->setAttribute('style', $style); | ||||||||||||||
| } | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| // Add border to table if missing (for visibility) | ||||||||||||||
| if (!$table->hasAttribute('border') || (int) $table->getAttribute('border') < 1) { | ||||||||||||||
| $table->setAttribute('border', '1'); | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| // Remove width/height styles only from table elements (table, td, th, tr) | ||||||||||||||
| $html = preg_replace('/(<(?:table|td|th|tr)\b[^>]*)\s+style\s*=\s*["\']([^"\']*)\bwidth\s*:\s*[^;"\'>]+;?([^"\']*)["\']/', '$1 style="$2$3"', $html); | ||||||||||||||
| $html = preg_replace('/(<(?:table|td|th|tr)\b[^>]*)\s+style\s*=\s*["\']([^"\']*)\bheight\s*:\s*[^;"\'>]+;?([^"\']*)["\']/', '$1 style="$2$3"', $html); | ||||||||||||||
| // Force table to 100% width for PDF (do this LAST) | ||||||||||||||
| $style = trim($table->getAttribute('style') . ';width:100%;', ';'); | ||||||||||||||
| $table->setAttribute('style', $style); | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| // Remove width/height attributes only from table elements (table, td, th, tr) | ||||||||||||||
| $html = preg_replace('/(<(?:table|td|th|tr)\b[^>]+)\s+width\s*=\s*["\']?[^"\'\s>]+["\']?/i', '$1', $html); | ||||||||||||||
| $html = preg_replace('/(<(?:table|td|th|tr)\b[^>]+)\s+height\s*=\s*["\']?[^"\'\s>]+["\']?/i', '$1', $html); | ||||||||||||||
| $output = ''; | ||||||||||||||
| foreach (iterator_to_array($wrapper->childNodes) as $child) { | ||||||||||||||
| $output .= $dom->saveHTML($child); | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| // Clean up empty style attributes and double spaces | ||||||||||||||
| $html = preg_replace('/\s+style\s*=\s*["\'][\s]*["\']/', '', $html); | ||||||||||||||
| $html = preg_replace('/\s+/', ' ', $html); | ||||||||||||||
| return $output; | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| // Add border to table if missing (for visibility) | ||||||||||||||
| if (!preg_match('/border\s*=\s*["\']?[1-9]/i', $html)) { | ||||||||||||||
| $html = preg_replace('/<table/i', '<table border="1"', $html, 1); | ||||||||||||||
| /** | ||||||||||||||
| * Remove the given CSS properties from an inline style declaration. | ||||||||||||||
| * | ||||||||||||||
| * @param $style string inline style attribute content | ||||||||||||||
| * @param $properties array lowercase property names to strip | ||||||||||||||
| * | ||||||||||||||
| * @return string | ||||||||||||||
| **/ | ||||||||||||||
| private function removeStyleProperties($style, array $properties) | ||||||||||||||
| { | ||||||||||||||
| if (trim($style) === '') { | ||||||||||||||
| return ''; | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| // Force table to 100% width for PDF (do this LAST) | ||||||||||||||
| $html = preg_replace('/<table([^>]*)>/i', '<table$1 style="width:100%">', $html, 1); | ||||||||||||||
| $kept = []; | ||||||||||||||
| foreach (explode(';', $style) as $declaration) { | ||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||
| $declaration = trim($declaration); | ||||||||||||||
| if ($declaration === '') { | ||||||||||||||
| continue; | ||||||||||||||
| } | ||||||||||||||
| $property = strtolower(trim(explode(':', $declaration, 2)[0])); | ||||||||||||||
| if (in_array($property, $properties, true)) { | ||||||||||||||
| continue; | ||||||||||||||
| } | ||||||||||||||
| $kept[] = $declaration; | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| return $html; | ||||||||||||||
| return implode('; ', $kept); | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| /** | ||||||||||||||
|
|
||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you add a unit test for
cleanTableHtml()with a nested-quoteurl('...')style value (case from !45825)? Verified locally that the old regex code produced a<table>with duplicateborder/styleattributes on this input, the actual crash mechanism, and that the new code doesn't; a test would pin the regression.