From 1e9626a7308a76eed1fb801899312ffa6bfdfdfd Mon Sep 17 00:00:00 2001 From: Jonathon Fowler Date: Mon, 19 Aug 2024 12:30:06 +1000 Subject: [PATCH] MDL-79819 assignfeedback_editpdf: safely handle bad jpg->pdf conversion --- .../editpdf/classes/document_services.php | 23 +++++++----- .../editpdf/tests/document_services_test.php | 33 ++++++++++++++++++ .../feedback/editpdf/tests/fixtures/heic.jpg | Bin 0 -> 605 bytes 3 files changed, 48 insertions(+), 8 deletions(-) create mode 100644 mod/assign/feedback/editpdf/tests/fixtures/heic.jpg diff --git a/mod/assign/feedback/editpdf/classes/document_services.php b/mod/assign/feedback/editpdf/classes/document_services.php index e93cf3ca5d8..193a6b6b51b 100644 --- a/mod/assign/feedback/editpdf/classes/document_services.php +++ b/mod/assign/feedback/editpdf/classes/document_services.php @@ -1033,7 +1033,7 @@ EOD; * @param int $attemptnumber Attempt Number * @param \stored_file $file file to save * @param null|array $size size of image - * @return \stored_file + * @return null|\stored_file * @throws \file_exception * @throws \stored_file_creation_exception */ @@ -1059,14 +1059,21 @@ EOD; $pdf->setImageScale(PDF_IMAGE_SCALE_RATIO); $pdf->AddPage($orientation); $pdf->SetAutoPageBreak(false); - // Width has to be define here to fit into A4 page. Otherwise the image will be inserted with original size. - if ($orientation == 'P') { - $pdf->Image('@' . $file->get_content(), 0, 0, 210); - } else { - $pdf->Image('@' . $file->get_content(), 0, 0, 297); + try { + // Width has to be defined here to fit into an A4 page, otherwise the image will be inserted with original size. + if ($orientation == 'P') { + $pdf->Image('@' . $file->get_content(), 0, 0, 210); + } else { + $pdf->Image('@' . $file->get_content(), 0, 0, 297); + } + $pdf->setPageMark(); + $pdf->save_pdf($tempfile); + } catch (\Exception $e) { + // Trim off the binary image data in the exception message for debugging output. + $exceptionmsg = strstr($e->getMessage(), '@', true) ?: $e->getMessage(); + debugging("Could not convert {$file->get_contenthash()} jpg to pdf: {$exceptionmsg}", DEBUG_ALL); + return null; } - $pdf->setPageMark(); - $pdf->save_pdf($tempfile); $filearea = self::TMP_JPG_TO_PDF_FILEAREA; $pdffile = self::save_file($assignment, $userid, $attemptnumber, $filearea, $tempfile); if (file_exists($tempfile)) { diff --git a/mod/assign/feedback/editpdf/tests/document_services_test.php b/mod/assign/feedback/editpdf/tests/document_services_test.php index 5040d7fc577..fbb790917ab 100644 --- a/mod/assign/feedback/editpdf/tests/document_services_test.php +++ b/mod/assign/feedback/editpdf/tests/document_services_test.php @@ -89,6 +89,39 @@ final class document_services_test extends \advanced_testcase { $this->assertEquals($file1->get_id(), $file2->get_id()); } + /** + * Test that save_jpg_to_pdf() method safely rejects a non-JPEG file with a JPEG extension. + */ + public function test_save_jpg_to_pdf_rejects_non_jpeg(): void { + $this->resetAfterTest(); + + $course = $this->getDataGenerator()->create_course(); + $generator = $this->getDataGenerator()->get_plugin_generator('mod_assign'); + $user = $this->getDataGenerator()->create_and_enrol($course); + + $assign = $this->create_instance($course, [ + 'assignsubmission_file_enabled' => 1, + 'assignsubmission_file_maxfiles' => 1, + 'assignsubmission_file_maxsizebytes' => 1024 * 1024, + ]); + $fileplugin = $assign->get_plugin_by_type('assignsubmission', 'file'); + $generator->create_submission([ + 'userid' => $user->id, + 'cmid' => $assign->get_course_module()->id, + 'file' => 'mod/assign/feedback/editpdf/tests/fixtures/heic.jpg', + ]); + $submission = $assign->get_user_submission($user->id, false); + $files = $fileplugin->get_files($submission, $user); + $this->assertEquals('image/jpeg', $files['/heic.jpg']->get_mimetype()); + + // Invoke the save_jpg_to_pdf method expecting there to be no exceptions. + $method = new \ReflectionMethod('\assignfeedback_editpdf\document_services', 'save_jpg_to_pdf'); + $retfile = $method->invoke(null, $assign, $user->id, 1, $files['/heic.jpg']); + $this->assertNull($retfile); + $this->assertDebuggingCalled("Could not convert {$files['/heic.jpg']->get_contenthash()} jpg to pdf: " . + "TCPDF ERROR: [Image] Unable to get the size of the image: ", DEBUG_ALL); + } + /** * Test that get_combined_document_for_attempt() method rotates the image only once. */ diff --git a/mod/assign/feedback/editpdf/tests/fixtures/heic.jpg b/mod/assign/feedback/editpdf/tests/fixtures/heic.jpg new file mode 100644 index 0000000000000000000000000000000000000000..d591e5b3c0f936b1d3bac12b29dae638ae5a8e71 GIT binary patch literal 605 zcmZQzV30{GsVvAy%}izhg51nBLl8SRGck>Ufzc^9wImTF4g`uBDLF+DCIdr3W^xIP z4Ws!AGD~v797Z6il$nzcHqym`fe{FSfH4h11DW4}SUEE@4`>MxfMi91oKz+#FQY8k z5X3x`Sx{5}q?cqCB)LwK?;}| zX6^uy405}e929|aI!=sWod}5qCXLI>f~LLl)LfUxB*g>P1R)%hfEw6c7!MY0P;vlT zB$8QNkP5btfguG*gRnqBW<@4QjG2dr2S|x!7UaUh3g}_xMkb);4F7Xe5kVQpK)}{43?bZ>1UG*COPq@$M0Qz ztIP6Y)5NdK{g__NcxUT-lk?cRuPk2<&h4}Dy4+}J#JlDD!bUllMy)%4?<)RS#n