MDL-67547 dataformat_pdf: method to convert images to supported format.
authorPaul Holden <paulh@moodle.com>
Mon, 27 Apr 2020 09:13:30 +0000 (10:13 +0100)
committerPaul Holden <paulh@moodle.com>
Mon, 25 May 2020 23:54:27 +0000 (00:54 +0100)
For Dataformats that support exporting HTML content, provide an API for
converting images within that content to something suitable for the format.

This fixes an issue with the PDF writer when it encountered a pluginfile.php
image, which it tried to request via HTTP without an active session. This
resulted in a 303 header returned by Moodle instead of the actual image,
causing an exception in the underlying TCPDF library.

dataformat/pdf/classes/writer.php
dataformat/pdf/tests/writer_test.php [new file with mode: 0644]
dataformat/upgrade.txt
lib/classes/dataformat/base.php

index 9768ffd..2203af5 100644 (file)
@@ -102,6 +102,23 @@ class writer extends \core\dataformat\base {
         return true;
     }
 
         return true;
     }
 
+    /**
+     * When exporting images, we need to return their Base64 encoded content. Otherwise TCPDF will create a HTTP
+     * request for them, which will lead to the login page (i.e. not the image it expects) and throw an exception
+     *
+     * Note: ideally we would copy the file to a temp location and return it's path, but a bug in TCPDF currently
+     * prevents that
+     *
+     * @param \stored_file $file
+     * @return string|null
+     */
+    protected function export_html_image_source(\stored_file $file): ?string {
+        // Set upper dimensions for embedded images.
+        $resizedimage = $file->resize_image(400, 300);
+
+        return '@' . base64_encode($resizedimage);
+    }
+
     /**
      * Write a single record
      *
     /**
      * Write a single record
      *
@@ -113,7 +130,15 @@ class writer extends \core\dataformat\base {
 
         $record = $this->format_record($record);
         foreach ($record as $cell) {
 
         $record = $this->format_record($record);
         foreach ($record as $cell) {
-            $rowheight = max($rowheight, $this->pdf->getStringHeight($this->colwidth, $cell, false, true, '', 1));
+            // We need to calculate the row height (accounting for any content). Unfortunately TCPDF doesn't provide an easy
+            // method to do that, so we create a second PDF inside a transaction, add cell content and use the largest cell by
+            // height. Solution similar to that at https://stackoverflow.com/a/1943096.
+            $pdf2 = clone $this->pdf;
+            $pdf2->startTransaction();
+            $pdf2->AddPage('L');
+            $pdf2->writeHTMLCell($this->colwidth, 0, '', '', $cell, 1, 1, false, true, 'L');
+            $rowheight = max($rowheight, $pdf2->getY() - $pdf2->getMargins()['top']);
+            $pdf2->rollbackTransaction();
         }
 
         $margins = $this->pdf->getMargins();
         }
 
         $margins = $this->pdf->getMargins();
diff --git a/dataformat/pdf/tests/writer_test.php b/dataformat/pdf/tests/writer_test.php
new file mode 100644 (file)
index 0000000..e1218a0
--- /dev/null
@@ -0,0 +1,74 @@
+<?php
+// This file is part of Moodle - http://moodle.org/
+//
+// Moodle is free software: you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+//
+// Moodle is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+//
+// You should have received a copy of the GNU General Public License
+// along with Moodle.  If not, see <http://www.gnu.org/licenses/>.
+
+/**
+ * Tests for the dataformat_pdf writer
+ *
+ * @package    dataformat_pdf
+ * @copyright  2020 Paul Holden <paulh@moodle.com>
+ * @license    http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
+ */
+
+namespace dataformat_pdf;
+
+use core\dataformat;
+use context_system;
+use html_writer;
+use moodle_url;
+
+/**
+ * Writer tests
+ *
+ * @package    dataformat_pdf
+ * @copyright  2020 Paul Holden <paulh@moodle.com>
+ * @license    http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
+ */
+class writer_testcase extends \advanced_testcase {
+
+    /**
+     * Test writing data whose content contains an image with pluginfile.php source
+     */
+    public function test_write_data_with_pluginfile_image(): void {
+        global $CFG;
+
+        $this->resetAfterTest(true);
+
+        $imagefixture = "{$CFG->dirroot}/lib/filestorage/tests/fixtures/testimage.jpg";
+        $image = get_file_storage()->create_file_from_pathname([
+            'contextid' => context_system::instance()->id,
+            'component' => 'dataformat_pdf',
+            'filearea'  => 'test',
+            'itemid'    => 0,
+            'filepath'  => '/',
+            'filename'  => basename($imagefixture),
+
+        ], $imagefixture);
+
+        $imageurl = moodle_url::make_pluginfile_url($image->get_contextid(), $image->get_component(), $image->get_filearea(),
+            $image->get_itemid(), $image->get_filepath(), $image->get_filename());
+
+        // Insert out test image into the data so it is exported.
+        $columns = ['animal', 'image'];
+        $row = ['cat', html_writer::img($imageurl->out(), 'My image')];
+
+        // Export to file. Assert that the exported file exists.
+        $exportfile = dataformat::write_data('My export', 'pdf', $columns, [$row]);
+        $this->assertFileExists($exportfile);
+
+        // The exported file should be a reasonable size (~275kb).
+        $this->assertGreaterThan(270000, filesize($exportfile));
+    }
+}
index cf4d09e..89eaa5b 100644 (file)
@@ -9,6 +9,13 @@ information provided here is intended especially for developers.
 * Calls to the following dataformat plugin methods have been removed:
   - write_header()
   - write_footer()
 * Calls to the following dataformat plugin methods have been removed:
   - write_header()
   - write_footer()
+* The following methods have been added to the base class to allow instances to define support for exporting
+  HTML content, with additional support for defining how images should be embedded:
+  - supports_html()
+  - export_html_image_source()
+* Dataformat writers should also call the following method to ensure data is properly formatted before being
+  written, which takes into account prior methods defining support for HTML:
+  - format_record()
 
 === 3.4 ===
 * In order to allow multiple sheets in an exported file the functions write_header() and write_footer() have
 
 === 3.4 ===
 * In order to allow multiple sheets in an exported file the functions write_header() and write_footer() have
index 9fb443b..454ed72 100644 (file)
@@ -163,9 +163,64 @@ abstract class base {
     protected function format_record($record): array {
         $record = (array)$record;
 
     protected function format_record($record): array {
         $record = (array)$record;
 
+        // If the dataformat supports export of HTML, we need to allow them to manage embedded images.
+        if ($this->supports_html()) {
+            $record = array_map([$this, 'replace_pluginfile_images'], $record);
+        }
+
         return $record;
     }
 
         return $record;
     }
 
+    /**
+     * Given a stored_file, return a suitable source attribute for an img element in the export (or null to use the original)
+     *
+     * @param \stored_file $file
+     * @return string|null
+     */
+    protected function export_html_image_source(\stored_file $file): ?string {
+        return null;
+    }
+
+    /**
+     * We need to locate all img tags within a given cell that match pluginfile URL's. Partly so the exported file will show
+     * the image without requiring the user is logged in; and also to prevent some of the dataformats requesting the file
+     * themselves, which is likely to fail due to them not having an active session
+     *
+     * @param string|null $content
+     * @return string
+     */
+    protected function replace_pluginfile_images(?string $content): string {
+        $content = (string)$content;
+
+        // Examine content to see if it contains any HTML image tags.
+        return preg_replace_callback('/(?<pre><img[^>]+src=")(?<source>[^"]*)(?<post>".*>)/i', function(array $matches) {
+            $source = $matches['source'];
+
+            // Now check if the image source looks like a pluginfile URL.
+            if (preg_match('/pluginfile.php\/(?<context>\d+)\/(?<component>[^\/]+)\/(?<filearea>[^\/]+)\/(?:(?<itemid>\d+)\/)?' .
+                    '(?<path>.*)/u', $source, $args)) {
+
+                $context = $args['context'];
+                $component = clean_param($args['component'], PARAM_COMPONENT);
+                $filearea = clean_param($args['filearea'], PARAM_AREA);
+                $itemid = $args['itemid'] ?: 0;
+                $path = clean_param(urldecode($args['path']), PARAM_PATH);
+
+                // Try and get the matching file from storage, allow the dataformat to define the replacement source.
+                $fullpath = "/{$context}/{$component}/{$filearea}/{$itemid}/{$path}";
+                if ($file = get_file_storage()->get_file_by_hash(sha1($fullpath))) {
+                    $exportsource = $this->export_html_image_source($file);
+
+                    if ($exportsource) {
+                        $source = $exportsource;
+                    }
+                }
+            }
+
+            return $matches['pre'] . $source . $matches['post'];
+        }, $content);
+    }
+
     /**
      * Write a single record
      *
     /**
      * Write a single record
      *