MDL-43593 Assign editpdf: More robust handling of errors from TCPDF.

This includes a check to see if there are 0 pages in the combined pdf,
catching exceptions and suppressing php warnings and errors from bad pdf files.

Also - Use TCPDF directly to check if pdfs are compatible. The previous check was
letting dodgy PDFs through which then failed at generation time. This way dodgy
pdfs will get run through ghostscript early and cleaned up.
This commit is contained in:
Damyon Wiese 2014-01-24 13:52:53 +08:00
parent b88bb98ab7
commit e205c062b1
3 changed files with 58 additions and 21 deletions

View file

@ -81,6 +81,9 @@ define('K_CELL_HEIGHT_RATIO', 1.25);
/** reduction factor for small font */
define('K_SMALL_RATIO', 2/3);
/** Throw exceptions from errors so they can be caught and recovered from. */
define('K_TCPDF_THROW_EXCEPTION_ERROR', true);
require_once(dirname(__FILE__).'/tcpdf/tcpdf.php');
/**

View file

@ -207,9 +207,17 @@ class document_services {
$tmpfile = $tmpdir . '/' . self::COMBINED_PDF_FILENAME;
@unlink($tmpfile);
try {
$pagecount = $pdf->combine_pdfs($compatiblepdfs, $tmpfile);
} catch (\Exception $e) {
debugging('TCPDF could not process the pdf files:' . $e->getMessage(), DEBUG_DEVELOPER);
// TCPDF does not recover from errors so we need to re-initialise the class.
$pagecount = 0;
}
if ($pagecount == 0) {
// We at least want a single blank page.
debugging('TCPDF did not produce a valid pdf:' . $tmpfile . '. Replacing with a blank pdf.', DEBUG_DEVELOPER);
$pdf = new pdf();
$pdf->AddPage();
@unlink($tmpfile);
$files = false;
@ -229,14 +237,27 @@ class document_services {
$fs->delete_area_files($record->contextid, $record->component, $record->filearea, $record->itemid);
// Detect corrupt generated pdfs and replace with a blank one.
if ($files) {
$pagecount = $pdf->load_pdf($tmpfile);
if ($pagecount <= 0) {
$files = false;
}
}
if (!$files) {
// This was a blank pdf.
unset($pdf);
$pdf = new pdf();
$content = $pdf->Output(self::COMBINED_PDF_FILENAME, 'S');
$file = $fs->create_file_from_string($record, $content);
} else {
// This was a combined pdf.
$file = $fs->create_file_from_pathname($record, $tmpfile);
@unlink($tmpfile);
// Test the generated file for correctness.
$compatiblepdf = pdf::ensure_pdf_compatible($file);
}
return $file;

View file

@ -78,6 +78,7 @@ class pdf extends \FPDI {
public function combine_pdfs($pdflist, $outfilename) {
raise_memory_limit(MEMORY_EXTRA);
$olddebug = error_reporting(0);
$this->setPageUnit('pt');
$this->setPrintHeader(false);
@ -97,6 +98,7 @@ class pdf extends \FPDI {
}
$this->save_pdf($outfilename);
error_reporting($olddebug);
return $totalpagecount;
}
@ -125,6 +127,7 @@ class pdf extends \FPDI {
*/
public function load_pdf($filename) {
raise_memory_limit(MEMORY_EXTRA);
$olddebug = error_reporting(0);
$this->setPageUnit('pt');
$this->scale = 72.0 / 100.0;
@ -138,6 +141,7 @@ class pdf extends \FPDI {
$this->pagecount = $this->setSourceFile($filename);
$this->filename = $filename;
error_reporting($olddebug);
return $this->pagecount;
}
@ -389,7 +393,9 @@ class pdf extends \FPDI {
* @param string $filename the filename for the PDF (including the full path)
*/
public function save_pdf($filename) {
$olddebug = error_reporting(0);
$this->Output($filename, 'F');
error_reporting($olddebug);
}
/**
@ -461,33 +467,26 @@ class pdf extends \FPDI {
* @return string path to copy or converted pdf (false == fail)
*/
public static function ensure_pdf_compatible(\stored_file $file) {
global $CFG;
$fp = $file->get_content_file_handle();
$ident = fread($fp, 10);
if (substr_compare('%PDF-', $ident, 0, 5) !== 0) {
return false; // This is not a PDF file at all.
}
$ident = substr($ident, 5); // Remove the '%PDF-' part.
$ident = explode('\x0A', $ident); // Truncate to first '0a' character.
list($major, $minor) = explode('.', $ident[0]); // Split the major / minor version.
$major = intval($major);
$minor = intval($minor);
if ($major == 0 || $minor == 0) {
return false; // Not a valid PDF version number.
}
$temparea = \make_temp_directory('assignfeedback_editpdf');
$hash = $file->get_contenthash(); // Use the contenthash to make sure the temp files have unique names.
$tempsrc = $temparea . "/src-$hash.pdf";
$tempdst = $temparea . "/dst-$hash.pdf";
$file->copy_content_to($tempsrc); // Copy the file.
if ($major = 1 && $minor<=4) {
// PDF is valid version - just create a copy we can use.
$file->copy_content_to($tempdst); // Copy the file.
return $tempdst;
$pdf = new pdf();
$pagecount = 0;
try {
$pagecount = $pdf->load_pdf($tempsrc);
} catch (\Exception $e) {
// PDF was not valid - try running it through ghostscript to clean it up.
$pagecount = 0;
}
if ($pagecount > 0) {
// Page is valid and can be read by tcpdf.
return $tempsrc;
}
$file->copy_content_to($tempsrc); // Copy the file.
$gsexec = \escapeshellarg(\get_config('assignfeedback_editpdf', 'gspath'));
$tempdstarg = \escapeshellarg($tempdst);
@ -500,6 +499,20 @@ class pdf extends \FPDI {
return false;
}
$pdf = new pdf();
$pagecount = 0;
try {
$pagecount = $pdf->load_pdf($tempdst);
} catch (\Exception $e) {
// PDF was not valid - try running it through ghostscript to clean it up.
$pagecount = 0;
}
if ($pagecount <= 0) {
@unlink($tempdst);
// Could not parse the converted pdf.
return false;
}
return $tempdst;
}