diff --git a/files/classes/archive_writer.php b/files/classes/archive_writer.php new file mode 100644 index 00000000000..415fca94239 --- /dev/null +++ b/files/classes/archive_writer.php @@ -0,0 +1,142 @@ +. + +/** + * Abstraction of general file archives. + * + * @package core_files + * @copyright 2020 Mark Nelson + * @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 + * @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; +} diff --git a/files/classes/local/archive_writer/file_writer_interface.php b/files/classes/local/archive_writer/file_writer_interface.php new file mode 100644 index 00000000000..6e2a175a26f --- /dev/null +++ b/files/classes/local/archive_writer/file_writer_interface.php @@ -0,0 +1,50 @@ +. + +/** + * Interface used by archives that write to files. + * + * @package core_files + * @copyright 2020 Mark Nelson + * @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 + * @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; +} diff --git a/files/classes/local/archive_writer/stream_writer_interface.php b/files/classes/local/archive_writer/stream_writer_interface.php new file mode 100644 index 00000000000..40435802329 --- /dev/null +++ b/files/classes/local/archive_writer/stream_writer_interface.php @@ -0,0 +1,43 @@ +. + +/** + * Interface used by archives that write to streams. + * + * @package core_files + * @copyright 2020 Mark Nelson + * @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 + * @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; +} diff --git a/files/classes/local/archive_writer/zip_writer.php b/files/classes/local/archive_writer/zip_writer.php new file mode 100644 index 00000000000..f6187f5bf36 --- /dev/null +++ b/files/classes/local/archive_writer/zip_writer.php @@ -0,0 +1,135 @@ +. + +/** + * Class used for creating ZIP archives. + * + * @package core_files + * @copyright 2020 Mark Nelson + * @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 + * @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); + } +} diff --git a/files/tests/archive_writer_test.php b/files/tests/archive_writer_test.php new file mode 100644 index 00000000000..544f58e21e2 --- /dev/null +++ b/files/tests/archive_writer_test.php @@ -0,0 +1,54 @@ +. + +/** + * Unit tests for core_files\local\archive_writer/zip_archive. + * + * @package core_files + * @category test + * @copyright 2020 Mark Nelson + * @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); + } +} diff --git a/files/tests/fixtures/awesome_file.txt b/files/tests/fixtures/awesome_file.txt new file mode 100644 index 00000000000..61193b64603 --- /dev/null +++ b/files/tests/fixtures/awesome_file.txt @@ -0,0 +1 @@ +Hey, this is an awesome text file. Hello! :) \ No newline at end of file diff --git a/files/tests/local/archive_writer/zip_writer_test.php b/files/tests/local/archive_writer/zip_writer_test.php new file mode 100644 index 00000000000..790d8d17d6d --- /dev/null +++ b/files/tests/local/archive_writer/zip_writer_test.php @@ -0,0 +1,187 @@ +. + +/** + * Unit tests for \core_files\local\archive_writer\zip_writer. + * + * @package core_files + * @category test + * @copyright 2020 Mark Nelson + * @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/'] + ]; + } +} diff --git a/mod/assign/locallib.php b/mod/assign/locallib.php index 825b7087361..ce652aa9933 100644 --- a/mod/assign/locallib.php +++ b/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(); } /**