Merge branch 'MDL-68533_master-clean_param' of git://github.com/mdjnelson/moodle

files/classes/archive_writer.php

@@ -0,0 +1,142 @@
+<?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/>.
+
+/**
+ * Abstraction of general file archives.
+ *
+ * @package   core_files
+ * @copyright 2020 Mark Nelson <mdjnelson@gmail.com>
+ * @license   http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
+ */
+
+namespace core_files;
+
+use core_files\local\archive_writer\file_writer_interface as file_writer_interface;
+use core_files\local\archive_writer\stream_writer_interface as stream_writer_interface;
+
+/**
+ * Each file archive type must extend this class.
+ *
+ * @package   core_files
+ * @copyright 2020 Mark Nelson <mdjnelson@gmail.com>
+ * @license   http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
+ */
+abstract class archive_writer {
+
+    /**
+     * The zip writer class.
+     */
+    public const ZIP_WRITER = 'zip_writer';
+
+    /**
+     * Returns the stream writer.
+     *
+     * @param string $filename
+     * @param string $type
+     * @return stream_writer_interface
+     */
+    public static function get_stream_writer(string $filename, string $type): stream_writer_interface {
+        $classname = self::get_classname_for_type($type);
+
+        if (!is_a($classname, stream_writer_interface::class, true)) {
+            throw new \InvalidArgumentException("{$type} does not support streaming");
+        }
+
+        return $classname::stream_instance($filename);
+    }
+
+    /**
+     * Returns the file writer.
+     *
+     * @param string $filepath
+     * @param string $type
+     * @return file_writer_interface
+     */
+    public static function get_file_writer(string $filepath, string $type): file_writer_interface {
+        $classname = self::get_classname_for_type($type);
+
+        if (!is_a($classname, file_writer_interface::class, true)) {
+            throw new \InvalidArgumentException("{$type} does not support writing to files");
+        }
+
+        return $classname::file_instance($filepath);
+    }
+
+    /**
+     * Sanitise the file path, removing any unsuitable characters.
+     *
+     * @param string $filepath
+     * @return string
+     */
+    public function sanitise_filepath(string $filepath): string {
+        return clean_param($filepath, PARAM_PATH);
+    }
+
+    /**
+     * Returns the class name for the type that was provided in get_file_writer().
+     *
+     * @param string $type
+     * @return string
+     */
+    protected static function get_classname_for_type(string $type): string {
+        return "core_files\local\archive_writer\\" . $type;
+    }
+
+    /**
+     * The archive_writer Constructor.
+     */
+    protected function __construct() {
+
+    }
+
+    /**
+     * Adds a file from a file path.
+     *
+     * @param string $name The path of file in archive (including directory).
+     * @param string $path The path to file on disk (note: paths should be encoded using
+     *                     UNIX-style forward slashes -- e.g '/path/to/some/file').
+     */
+    abstract public function add_file_from_filepath(string $name, string $path): void;
+
+    /**
+     * Adds a file from a string.
+     *
+     * @param string $name The path of file in archive (including directory).
+     * @param string $data The contents of file
+     */
+    abstract public function add_file_from_string(string $name, string $data): void;
+
+    /**
+     * Adds a file from a stream.
+     *
+     * @param string $name The path of file in archive (including directory).
+     * @param resource $stream The contents of file as a stream resource
+     */
+    abstract public function add_file_from_stream(string $name, $stream): void;
+
+    /**
+     * Adds a stored_file to archive.
+     *
+     * @param string $name The path of file in archive (including directory).
+     * @param \stored_file $file
+     */
+    abstract public function add_file_from_stored_file(string $name, \stored_file $file): void;
+
+    /**
+     * Finish writing the zip footer.
+     */
+    abstract public function finish(): void;
+}

files/classes/local/archive_writer/file_writer_interface.php

@@ -0,0 +1,50 @@
+<?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/>.
+
+/**
+ * Interface used by archives that write to files.
+ *
+ * @package   core_files
+ * @copyright 2020 Mark Nelson <mdjnelson@gmail.com>
+ * @license   http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
+ */
+
+namespace core_files\local\archive_writer;
+
+/**
+ * Interface used by archives that write to files.
+ *
+ * @package   core_files
+ * @copyright 2020 Mark Nelson <mdjnelson@gmail.com>
+ * @license   http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
+ */
+interface file_writer_interface {
+
+    /**
+     * Return the file instance.
+     *
+     * @param string $filename
+     * @return static
+     */
+    public static function file_instance(string $filename): self;
+
+    /**
+     * Get the path of the zip.
+     *
+     * @return string
+     */
+    public function get_path_to_zip(): string;
+}

files/classes/local/archive_writer/stream_writer_interface.php

@@ -0,0 +1,43 @@
+<?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/>.
+
+/**
+ * Interface used by archives that write to streams.
+ *
+ * @package   core_files
+ * @copyright 2020 Mark Nelson <mdjnelson@gmail.com>
+ * @license   http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
+ */
+
+namespace core_files\local\archive_writer;
+
+/**
+ * Interface used by archives that write to streams.
+ *
+ * @package   core_files
+ * @copyright 2020 Mark Nelson <mdjnelson@gmail.com>
+ * @license   http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
+ */
+interface stream_writer_interface {
+
+    /**
+     * Return the stream instance.
+     *
+     * @param string $filename
+     * @return static
+     */
+    public static function stream_instance(string $filename): self;
+}

files/classes/local/archive_writer/zip_writer.php

@@ -0,0 +1,135 @@
+<?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/>.
+
+/**
+ * Class used for creating ZIP archives.
+ *
+ * @package   core_files
+ * @copyright 2020 Mark Nelson <mdjnelson@gmail.com>
+ * @license   http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
+ */
+
+namespace core_files\local\archive_writer;
+
+use ZipStream\Option\Archive;
+use ZipStream\ZipStream;
+use core_files\archive_writer;
+use core_files\local\archive_writer\file_writer_interface as file_writer_interface;
+use core_files\local\archive_writer\stream_writer_interface as stream_writer_interface;
+
+/**
+ * Class used for creating ZIP archives.
+ *
+ * @package   core_files
+ * @copyright 2020 Mark Nelson <mdjnelson@gmail.com>
+ * @license   http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
+ */
+class zip_writer extends archive_writer implements file_writer_interface, stream_writer_interface {
+
+    /**
+     * @var resource File resource for the file handle for a file-based zip stream
+     */
+    private $zipfilehandle = null;
+
+    /**
+     * @var String The location of the zip file.
+     */
+    private $zipfilepath = null;
+
+    /**
+     * @var ZipStream The zip stream.
+     */
+    private $archive;
+
+    /**
+     * The zip_writer constructor.
+     *
+     * @param ZipStream $archive
+     */
+    protected function __construct(ZipStream $archive) {
+        parent::__construct();
+        $this->archive = $archive;
+    }
+
+    public static function stream_instance(string $filename): stream_writer_interface {
+        $options = new Archive();
+        $options->setSendHttpHeaders(true);
+        $options->setContentDisposition('attachment');
+        $options->setContentType('application/x-zip');
+        $zipwriter = new ZipStream($filename, $options);
+
+        return new static($zipwriter);
+    }
+
+    public static function file_instance(string $filename): file_writer_interface {
+        $dir = make_request_directory();
+        $filepath = "$dir/$filename";
+        $fh = fopen($filepath, 'w');
+
+        $exportoptions = new Archive();
+        $exportoptions->setOutputStream($fh);
+        $exportoptions->setSendHttpHeaders(false);
+        $zipstream = new ZipStream($filename, $exportoptions);
+
+        $zipwriter = new static($zipstream);
+        // ZipStream only takes a file handle resource.
+        // It does not close this resource itself, and it does not know the location of this resource on disk.
+        // Store references to the filehandle, and the location of the filepath in the new class so that the `finish()`
+        // function can close the fh, and move the temporary file into place.
+        // The filehandle must be closed when finishing the archive. ZipStream does not close it automatically.
+        $zipwriter->zipfilehandle = $fh;
+        $zipwriter->zipfilepath = $filepath;
+
+        return $zipwriter;
+    }
+
+    public function add_file_from_filepath(string $name, string $path): void {
+        $this->archive->addFileFromPath($this->sanitise_filepath($name), $path);
+    }
+
+    public function add_file_from_string(string $name, string $data): void {
+        $this->archive->addFile($this->sanitise_filepath($name), $data);
+    }
+
+    public function add_file_from_stream(string $name, $stream): void {
+        $this->archive->addFileFromStream($this->sanitise_filepath($name), $stream);
+        fclose($stream);
+    }
+
+    public function add_file_from_stored_file(string $name, \stored_file $file): void {
+        $filehandle = $file->get_content_file_handle();
+        $this->archive->addFileFromStream($this->sanitise_filepath($name), $filehandle);
+        fclose($filehandle);
+    }
+
+    public function finish(): void {
+        $this->archive->finish();
+
+        if ($this->zipfilehandle) {
+            fclose($this->zipfilehandle);
+        }
+    }
+
+    public function get_path_to_zip(): string {
+        return $this->zipfilepath;
+    }
+
+    public function sanitise_filepath(string $filepath): string {
+        $filepath = parent::sanitise_filepath($filepath);
+
+        return \ZipStream\File::filterFilename($filepath);
+    }
+}

files/tests/archive_writer_test.php

@@ -0,0 +1,54 @@
+<?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/>.
+
+/**
+ * Unit tests for core_files\local\archive_writer/zip_archive.
+ *
+ * @package core_files
+ * @category test
+ * @copyright 2020 Mark Nelson <mdjnelson@gmail.com>
+ * @license http://www.gnu.org/copyleft/gpl.html GNU Public License
+ */
+
+namespace core_files;
+
+use advanced_testcase;
+use core_files\local\archive_writer\zip_writer;
+
+/**
+ * Unit tests for \core_files\archive_writer.
+ *
+ * @coversDefaultClass \core_files\archive_writer
+ */
+class archive_writer_testcase extends advanced_testcase {
+
+    /**
+     * Test get_file_writer().
+     */
+    public function test_get_file_writer(): void {
+        $zipwriter = archive_writer::get_file_writer('file.zip', archive_writer::ZIP_WRITER);
+        $this->assertInstanceOf(zip_writer::class, $zipwriter);
+        $this->assertTrue(file_exists($zipwriter->get_path_to_zip()));
+    }
+
+    /**
+     * Test get_stream_writer().
+     */
+    public function test_get_stream_writer(): void {
+        $zipwriter = archive_writer::get_stream_writer('path/to/file.txt', archive_writer::ZIP_WRITER);
+        $this->assertInstanceOf(zip_writer::class, $zipwriter);
+    }
+}

files/tests/fixtures/awesome_file.txt

@@ -0,0 +1 @@
+Hey, this is an awesome text file. Hello! :)

files/tests/local/archive_writer/zip_writer_test.php

@@ -0,0 +1,187 @@
+<?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/>.
+
+/**
+ * Unit tests for \core_files\local\archive_writer\zip_writer.
+ *
+ * @package core_files
+ * @category test
+ * @copyright 2020 Mark Nelson <mdjnelson@gmail.com>
+ * @license http://www.gnu.org/copyleft/gpl.html GNU Public License
+ */
+
+namespace core_files\local\archive_writer;
+
+use advanced_testcase;
+use context_module;
+use core_files\archive_writer;
+use ZipArchive;
+
+/**
+ * Unit tests for \core_files\local\archive_writer\zip_writer.
+ *
+ * @coversDefaultClass \core_files\local\archive_writer\zip_writer
+ */
+class zip_writer_testcase extends advanced_testcase {
+
+    /**
+     * Test add_file_from_filepath().
+     */
+    public function test_add_file_from_filepath(): void {
+        global $CFG;
+
+        $pathtofileinzip = '/some/made/up/name.txt';
+        $filetoadd = $CFG->dirroot . '/files/tests/fixtures/awesome_file.txt';
+
+        $zipwriter = archive_writer::get_file_writer('test.zip', archive_writer::ZIP_WRITER);
+        $zipwriter->add_file_from_filepath($pathtofileinzip, $filetoadd);
+        $zipwriter->finish();
+
+        $pathtozip = $zipwriter->get_path_to_zip();
+        $zip = new ZipArchive();
+        $opened = $zip->open($pathtozip);
+        $this->assertTrue($opened);
+
+        $pathtofileinzip = $zipwriter->sanitise_filepath($pathtofileinzip);
+
+        $this->assertEquals("Hey, this is an awesome text file. Hello! :)", $zip->getFromName($pathtofileinzip));
+    }
+
+    /**
+     * Test add_file_from_string().
+     */
+    public function test_add_file_from_string(): void {
+        $pathtofileinzip = "/path/to/my/awesome/file.txt";
+        $mycontent = "This is some real awesome content, ya dig?";
+
+        $zipwriter = archive_writer::get_file_writer('test.zip', archive_writer::ZIP_WRITER);
+        $zipwriter->add_file_from_string($pathtofileinzip, $mycontent);
+        $zipwriter->finish();
+
+        $pathtozip = $zipwriter->get_path_to_zip();
+        $zip = new ZipArchive();
+        $opened = $zip->open($pathtozip);
+        $this->assertTrue($opened);
+
+        $pathtofileinzip = $zipwriter->sanitise_filepath($pathtofileinzip);
+
+        $this->assertEquals($mycontent, $zip->getFromName($pathtofileinzip));
+    }
+
+    /**
+     * Test add_file_from_stream().
+     */
+    public function test_add_file_from_stream(): void {
+        $this->resetAfterTest(true);
+        $this->setAdminUser();
+
+        $course = $this->getDataGenerator()->create_course();
+        $assign = $this->getDataGenerator()->create_module('assign', ['course' => $course->id]);
+
+        // Add a file to the intro.
+        $filerecord = [
+            'contextid' => context_module::instance($assign->cmid)->id,
+            'component' => 'mod_assign',
+            'filearea'  => 'intro',
+            'itemid'    => 0,
+            'filepath'  => '/',
+            'filename'  => 'fileintro.txt',
+        ];
+        $fs = get_file_storage();
+        $storedfile = $fs->create_file_from_string($filerecord, 'Contents for the assignment, yeow!');
+
+        $pathtofileinzip = $storedfile->get_filepath() . $storedfile->get_filename();
+
+        $zipwriter = archive_writer::get_file_writer('test.zip', archive_writer::ZIP_WRITER);
+        $zipwriter->add_file_from_stream($pathtofileinzip, $storedfile->get_content_file_handle());
+        $zipwriter->finish();
+
+        $pathtozip = $zipwriter->get_path_to_zip();
+        $zip = new ZipArchive();
+        $opened = $zip->open($pathtozip);
+        $this->assertTrue($opened);
+
+        $pathtofileinzip = $zipwriter->sanitise_filepath($pathtofileinzip);
+
+        $this->assertEquals($storedfile->get_content(), $zip->getFromName($pathtofileinzip));
+    }
+
+    /**
+     * Test add_file_from_stored_file().
+     */
+    public function test_add_file_from_stored_file(): void {
+        $this->resetAfterTest(true);
+        $this->setAdminUser();
+
+        $course = $this->getDataGenerator()->create_course();
+        $assign = $this->getDataGenerator()->create_module('assign', ['course' => $course->id]);
+
+        // Add a file to the intro.
+        $filerecord = [
+            'contextid' => context_module::instance($assign->cmid)->id,
+            'component' => 'mod_assign',
+            'filearea'  => 'intro',
+            'itemid'    => 0,
+            'filepath'  => '/',
+            'filename'  => 'fileintro.txt',
+        ];
+        $fs = get_file_storage();
+        $storedfile = $fs->create_file_from_string($filerecord, 'Contents for the assignment, yeow!');
+
+        $pathtofileinzip = $storedfile->get_filepath() . $storedfile->get_filename();
+
+        $zipwriter = archive_writer::get_file_writer('test.zip', archive_writer::ZIP_WRITER);
+        $zipwriter->add_file_from_stored_file($pathtofileinzip, $storedfile);
+        $zipwriter->finish();
+
+        $pathtozip = $zipwriter->get_path_to_zip();
+        $zip = new ZipArchive();
+        $opened = $zip->open($pathtozip);
+        $this->assertTrue($opened);
+
+        $pathtofileinzip = $zipwriter->sanitise_filepath($pathtofileinzip);
+
+        $this->assertEquals($storedfile->get_content(), $zip->getFromName($pathtofileinzip));
+    }
+
+    /**
+     * Test sanitise_filepath().
+     *
+     * @param string $providedfilepath The provided file path.
+     * @param string $expectedfilepath The expected file path.
+     * @dataProvider sanitise_filepath_provider
+     */
+    public function test_sanitise_filepath(string $providedfilepath, string $expectedfilepath): void {
+        $zipwriter = archive_writer::get_stream_writer('path/to/file.txt', archive_writer::ZIP_WRITER);
+        $this->assertEquals($expectedfilepath, $zipwriter->sanitise_filepath($providedfilepath));
+    }
+
+    /**
+     * Data provider for test_sanitise_filepath.
+     *
+     * @return array
+     */
+    public function sanitise_filepath_provider(): array {
+        return [
+            ['a../../file/path', 'a../file/path'],
+            ['a./file/path', 'a./file/path'],
+            ['../file/path', 'file/path'],
+            ['foo/bar/', 'foo/bar/'],
+            ['\\\\\\a\\\\\\file\\\\\\path', 'a/file/path'],
+            ['//a//file/////path////', 'a/file/path/']
+        ];
+    }
+}

mod/assign/locallib.php

@@ -2505,27 +2505,6 @@ class assign {
         return $useridlist;
     }
 
-    /**
-     * Generate zip file from array of given files.
-     *
-     * @param array $filesforzipping - array of files to pass into archive_to_pathname.
-     *                                 This array is indexed by the final file name and each
-     *                                 element in the array is an instance of a stored_file object.
-     * @return path of temp file - note this returned file does
-     *         not have a .zip extension - it is a temp file.
-     */
-    protected function pack_files($filesforzipping) {
-        global $CFG;
-        // Create path for new zip file.
-        $tempzip = tempnam($CFG->tempdir . '/', 'assignment_');
-        // Zip files.
-        $zipper = new zip_packer();
-        if ($zipper->archive_to_pathname($filesforzipping, $tempzip)) {
-            return $tempzip;
-        }
-        return false;
-    }
-
     /**
      * Finds all assignment notifications that have yet to be mailed out, and mails them.
      *
@@ -3660,13 +3639,26 @@ class assign {
                                                                     'action'=>'grading'));
             $result .= $this->get_renderer()->continue_button($url);
             $result .= $this->view_footer();
-        } else if ($zipfile = $this->pack_files($filesforzipping)) {
-            \mod_assign\event\all_submissions_downloaded::create_from_assign($this)->trigger();
-            // Send file and delete after sending.
-            send_temp_file($zipfile, $filename);
-            // We will not get here - send_temp_file calls exit.
+
+            return $result;
         }
-        return $result;
+
+        // Log zip as downloaded.
+        \mod_assign\event\all_submissions_downloaded::create_from_assign($this)->trigger();
+
+        // Close the session.
+        \core\session\manager::write_close();
+
+        $zipwriter = \core_files\archive_writer::get_stream_writer($filename, \core_files\archive_writer::ZIP_WRITER);
+
+        // Stream the files into the zip.
+        foreach ($filesforzipping as $pathinzip => $storedfile) {
+            $zipwriter->add_file_from_stored_file($pathinzip, $storedfile);
+        }
+
+        // Finish the archive.
+        $zipwriter->finish();
+        exit();
     }
 
     /**