diff --git a/lib/moodlelib.php b/lib/moodlelib.php index 04b0022b4db..e1b368ab397 100644 --- a/lib/moodlelib.php +++ b/lib/moodlelib.php @@ -8267,9 +8267,10 @@ function shorten_text($text, $ideal=30, $exact = false, $ending='...') { * * @param string $filename file name * @param int $length ideal string length + * @param bool $includehash Whether to include a file hash in the shortened version. This ensures uniqueness. * @return string $shortened shortened file name */ -function shorten_filename($filename, $length = MAX_FILENAME_SIZE) { +function shorten_filename($filename, $length = MAX_FILENAME_SIZE, $includehash = false) { $shortened = $filename; // Extract a part of the filename if it's char size exceeds the ideal string length. if (core_text::strlen($filename) > $length) { @@ -8278,15 +8279,36 @@ function shorten_filename($filename, $length = MAX_FILENAME_SIZE) { $extension = pathinfo($filename, PATHINFO_EXTENSION); if ($extension && !empty($mimetypes[$extension])) { $basename = pathinfo($filename, PATHINFO_FILENAME); - $shortened = core_text::substr($basename, 0, $length); + $hash = empty($includehash) ? '' : ' - ' . substr(sha1($basename), 0, 10); + $shortened = core_text::substr($basename, 0, $length - strlen($hash)) . $hash; $shortened .= '.' . $extension; } else { - $shortened = core_text::substr($filename, 0, $length); + $hash = empty($includehash) ? '' : ' - ' . substr(sha1($filename), 0, 10); + $shortened = core_text::substr($filename, 0, $length - strlen($hash)) . $hash; } } return $shortened; } +/** + * Shortens a given array of filenames by removing characters positioned after the ideal string length. + * + * @param array $path The paths to reduce the length. + * @param int $length Ideal string length + * @param bool $includehash Whether to include a file hash in the shortened version. This ensures uniqueness. + * @return array $result Shortened paths in array. + */ +function shorten_filenames(array $path, $length = MAX_FILENAME_SIZE, $includehash = false) { + $result = null; + + $result = array_reduce($path, function($carry, $singlepath) use ($length, $includehash) { + $carry[] = shorten_filename($singlepath, $length, $includehash); + return $carry; + }, []); + + return $result; +} + /** * Given dates in seconds, how many weeks is the date from startdate * The first week is 1, the second 2 etc ... diff --git a/lib/tests/moodlelib_test.php b/lib/tests/moodlelib_test.php index 87f59e82233..80dc799168d 100644 --- a/lib/tests/moodlelib_test.php +++ b/lib/tests/moodlelib_test.php @@ -1007,27 +1007,238 @@ class core_moodlelib_testcase extends advanced_testcase { shorten_text($text, 1)); } - public function test_shorten_filename() { - // Test filename that contains more than 100 characters. + /** + * Provider for long filenames and its expected result, with and without hash. + * + * @return array of ($filename, $length, $expected, $includehash) + */ + public function shorten_filename_provider() { $filename = 'sed ut perspiciatis unde omnis iste natus error sit voluptatem accusantium doloremque laudantium totam rem'; - $this->assertSame('sed ut perspiciatis unde omnis iste natus error sit voluptatem accusantium doloremque laudantium tot', - shorten_filename($filename)); - // Filename contains extension. - $this->assertSame('sed ut perspiciatis unde omnis iste natus error sit voluptatem accusantium doloremque laudantium tot.zip', - shorten_filename($filename . '.zip')); - // Limit filename to 50 chars. - $this->assertSame('sed ut perspiciatis unde omnis iste natus error si', - shorten_filename($filename, 50)); - $this->assertSame('sed ut perspiciatis unde omnis iste natus error si.zip', - shorten_filename($filename . '.zip', 50)); + $shortfilename = 'sed ut perspiciatis unde omnis iste natus error sit voluptatem accusantium doloremque'; - // Test filename that contains less than 100 characters. - $filename = 'sed ut perspiciatis unde omnis iste natus error sit voluptatem accusantium doloremque'; - $this->assertSame('sed ut perspiciatis unde omnis iste natus error sit voluptatem accusantium doloremque', - shorten_filename($filename)); - // Filename contains extension. - $this->assertSame('sed ut perspiciatis unde omnis iste natus error sit voluptatem accusantium doloremque.zip', - shorten_filename($filename . '.zip')); + return [ + 'More than 100 characters' => [ + $filename, + null, + 'sed ut perspiciatis unde omnis iste natus error sit voluptatem accusantium doloremque laudantium tot', + false, + ], + 'More than 100 characters with hash' => [ + $filename, + null, + 'sed ut perspiciatis unde omnis iste natus error sit voluptatem accusantium doloremque l - 3bec1da8b8', + true, + ], + 'More than 100 characters with extension' => [ + "{$filename}.zip", + null, + 'sed ut perspiciatis unde omnis iste natus error sit voluptatem accusantium doloremque laudantium tot.zip', + false, + ], + 'More than 100 characters with extension and hash' => [ + "{$filename}.zip", + null, + 'sed ut perspiciatis unde omnis iste natus error sit voluptatem accusantium doloremque l - 3bec1da8b8.zip', + true, + ], + 'Limit filename to 50 chars' => [ + $filename, + 50, + 'sed ut perspiciatis unde omnis iste natus error si', + false, + ], + 'Limit filename to 50 chars with hash' => [ + $filename, + 50, + 'sed ut perspiciatis unde omnis iste n - 3bec1da8b8', + true, + ], + 'Limit filename to 50 chars with extension' => [ + "{$filename}.zip", + 50, + 'sed ut perspiciatis unde omnis iste natus error si.zip', + false, + ], + 'Limit filename to 50 chars with extension and hash' => [ + "{$filename}.zip", + 50, + 'sed ut perspiciatis unde omnis iste n - 3bec1da8b8.zip', + true, + ], + 'Test filename that contains less than 100 characters' => [ + $shortfilename, + null, + $shortfilename, + false, + ], + 'Test filename that contains less than 100 characters and hash' => [ + $shortfilename, + null, + $shortfilename, + true, + ], + 'Test filename that contains less than 100 characters with extension' => [ + "{$shortfilename}.zip", + null, + "{$shortfilename}.zip", + false, + ], + 'Test filename that contains less than 100 characters with extension and hash' => [ + "{$shortfilename}.zip", + null, + "{$shortfilename}.zip", + true, + ], + ]; + } + + /** + * Test the {@link shorten_filename()} method. + * + * @dataProvider shorten_filename_provider + * + * @param string $filename + * @param int $length + * @param string $expected + * @param boolean $includehash + */ + public function test_shorten_filename($filename, $length, $expected, $includehash) { + if (null === $length) { + $length = MAX_FILENAME_SIZE; + } + + $this->assertSame($expected, shorten_filename($filename, $length, $includehash)); + } + + /** + * Provider for long filenames and its expected result, with and without hash. + * + * @return array of ($filename, $length, $expected, $includehash) + */ + public function shorten_filenames_provider() { + $shortfilename = 'sed ut perspiciatis unde omnis iste natus error sit voluptatem accusantium doloremque'; + $longfilename = 'sed ut perspiciatis unde omnis iste natus error sit voluptatem accusantium doloremque laudantium totam rem'; + $extfilename = $longfilename.'.zip'; + $expected = 'sed ut perspiciatis unde omnis iste natus error sit voluptatem accusantium doloremque laudantium tot'; + $expectedwithhash = 'sed ut perspiciatis unde omnis iste natus error sit voluptatem accusantium doloremque l - 3bec1da8b8'; + $expectedext = 'sed ut perspiciatis unde omnis iste natus error sit voluptatem accusantium doloremque laudantium tot.zip'; + $expectedextwithhash = 'sed ut perspiciatis unde omnis iste natus error sit voluptatem accusantium doloremque l - 3bec1da8b8.zip'; + $expected50 = 'sed ut perspiciatis unde omnis iste natus error si'; + $expected50withhash = 'sed ut perspiciatis unde omnis iste n - 3bec1da8b8'; + $expected50ext = 'sed ut perspiciatis unde omnis iste natus error si.zip'; + $expected50extwithhash = 'sed ut perspiciatis unde omnis iste n - 3bec1da8b8.zip'; + $expected50short = 'sed ut perspiciatis unde omnis iste n - 5fb6543490'; + + return [ + 'Empty array without hash' => [ + [], + null, + [], + false, + ], + 'Empty array with hash' => [ + [], + null, + [], + true, + ], + 'Array with less than 100 characters' => [ + [$shortfilename, $shortfilename, $shortfilename], + null, + [$shortfilename, $shortfilename, $shortfilename], + false, + ], + 'Array with more than 100 characters without hash' => [ + [$longfilename, $longfilename, $longfilename], + null, + [$expected, $expected, $expected], + false, + ], + 'Array with more than 100 characters with hash' => [ + [$longfilename, $longfilename, $longfilename], + null, + [$expectedwithhash, $expectedwithhash, $expectedwithhash], + true, + ], + 'Array with more than 100 characters with extension' => [ + [$extfilename, $extfilename, $extfilename], + null, + [$expectedext, $expectedext, $expectedext], + false, + ], + 'Array with more than 100 characters with extension and hash' => [ + [$extfilename, $extfilename, $extfilename], + null, + [$expectedextwithhash, $expectedextwithhash, $expectedextwithhash], + true, + ], + 'Array with more than 100 characters mix (short, long, with extension) without hash' => [ + [$shortfilename, $longfilename, $extfilename], + null, + [$shortfilename, $expected, $expectedext], + false, + ], + 'Array with more than 100 characters mix (short, long, with extension) with hash' => [ + [$shortfilename, $longfilename, $extfilename], + null, + [$shortfilename, $expectedwithhash, $expectedextwithhash], + true, + ], + 'Array with less than 50 characters without hash' => [ + [$longfilename, $longfilename, $longfilename], + 50, + [$expected50, $expected50, $expected50], + false, + ], + 'Array with less than 50 characters with hash' => [ + [$longfilename, $longfilename, $longfilename], + 50, + [$expected50withhash, $expected50withhash, $expected50withhash], + true, + ], + 'Array with less than 50 characters with extension' => [ + [$extfilename, $extfilename, $extfilename], + 50, + [$expected50ext, $expected50ext, $expected50ext], + false, + ], + 'Array with less than 50 characters with extension and hash' => [ + [$extfilename, $extfilename, $extfilename], + 50, + [$expected50extwithhash, $expected50extwithhash, $expected50extwithhash], + true, + ], + 'Array with less than 50 characters mix (short, long, with extension) without hash' => [ + [$shortfilename, $longfilename, $extfilename], + 50, + [$expected50, $expected50, $expected50ext], + false, + ], + 'Array with less than 50 characters mix (short, long, with extension) with hash' => [ + [$shortfilename, $longfilename, $extfilename], + 50, + [$expected50short, $expected50withhash, $expected50extwithhash], + true, + ], + ]; + } + + /** + * Test the {@link shorten_filenames()} method. + * + * @dataProvider shorten_filenames_provider + * + * @param string $filenames + * @param int $length + * @param string $expected + * @param boolean $includehash + */ + public function test_shorten_filenames($filenames, $length, $expected, $includehash) { + if (null === $length) { + $length = MAX_FILENAME_SIZE; + } + + $this->assertSame($expected, shorten_filenames($filenames, $length, $includehash)); } public function test_usergetdate() { diff --git a/privacy/classes/local/request/moodle_content_writer.php b/privacy/classes/local/request/moodle_content_writer.php index de607ccdf82..4e8f1009a56 100644 --- a/privacy/classes/local/request/moodle_content_writer.php +++ b/privacy/classes/local/request/moodle_content_writer.php @@ -244,7 +244,7 @@ class moodle_content_writer implements content_writer { $path = []; $contexts = array_reverse($this->context->get_parent_contexts(true)); foreach ($contexts as $context) { - $path[] = clean_param($context->get_context_name(), PARAM_FILE); + $path[] = shorten_filename(clean_param($context->get_context_name(), PARAM_FILE), MAX_FILENAME_SIZE, true); } return $path; @@ -258,6 +258,9 @@ class moodle_content_writer implements content_writer { * @return string The fully-qualfiied file path. */ protected function get_path(array $subcontext, string $name) : string { + $subcontext = shorten_filenames($subcontext, MAX_FILENAME_SIZE, true); + $name = shorten_filename($name, MAX_FILENAME_SIZE, true); + // Combine the context path, and the subcontext data. $path = array_merge( $this->get_context_path(), diff --git a/privacy/tests/moodle_content_writer_test.php b/privacy/tests/moodle_content_writer_test.php index b683348186f..a045829e89b 100644 --- a/privacy/tests/moodle_content_writer_test.php +++ b/privacy/tests/moodle_content_writer_test.php @@ -913,6 +913,151 @@ class moodle_content_writer_test extends advanced_testcase { ]; } + /** + * Test that exported data is shortened when exceeds the limit. + * + * @dataProvider long_filename_provider + * @param string $longtext + * @param string $expected + * @param string $text + */ + public function test_export_data_long_filename($longtext, $expected, $text) { + $context = \context_system::instance(); + $subcontext = [$longtext]; + $data = (object) ['key' => $text]; + + $writer = $this->get_writer_instance() + ->set_context($context) + ->export_data($subcontext, $data); + + $fileroot = $this->fetch_exported_content($writer); + + $contextpath = $this->get_context_path($context, $subcontext, 'data.json'); + $expectedpath = 'System/'.$expected.'/data.json'; + $this->assertEquals($expectedpath, $contextpath); + + $json = $fileroot->getChild($contextpath)->getContent(); + $this->assertRegExp("/$text/", $json); + + $expanded = json_decode($json); + $this->assertEquals($data, $expanded); + } + + /** + * Test that exported related data is shortened when exceeds the limit. + * + * @dataProvider long_filename_provider + * @param string $longtext + * @param string $expected + * @param string $text + */ + public function test_export_related_data_long_filename($longtext, $expected, $text) { + $context = \context_system::instance(); + $subcontext = [$longtext]; + $data = (object) ['key' => $text]; + + $writer = $this->get_writer_instance() + ->set_context($context) + ->export_related_data($subcontext, 'name', $data); + + $fileroot = $this->fetch_exported_content($writer); + + $contextpath = $this->get_context_path($context, $subcontext, 'name.json'); + $expectedpath = 'System/'.$expected.'/name.json'; + $this->assertEquals($expectedpath, $contextpath); + + $json = $fileroot->getChild($contextpath)->getContent(); + $this->assertRegExp("/$text/", $json); + + $expanded = json_decode($json); + $this->assertEquals($data, $expanded); + } + + /** + * Test that exported metadata is shortened when exceeds the limit. + * + * @dataProvider long_filename_provider + * @param string $longtext + * @param string $expected + * @param string $text + */ + public function test_export_metadata_long_filename($longtext, $expected, $text) { + $context = \context_system::instance(); + $subcontext = [$longtext]; + $data = (object) ['key' => $text]; + + $writer = $this->get_writer_instance() + ->set_context($context) + ->export_metadata($subcontext, $text, $text, $text); + + $fileroot = $this->fetch_exported_content($writer); + + $contextpath = $this->get_context_path($context, $subcontext, 'metadata.json'); + $expectedpath = 'System/'.$expected.'/metadata.json'; + $this->assertEquals($expectedpath, $contextpath); + + $json = $fileroot->getChild($contextpath)->getContent(); + $this->assertRegExp("/$text.*$text.*$text/", $json); + + $expanded = json_decode($json); + $this->assertTrue(isset($expanded->$text)); + $this->assertEquals($text, $expanded->$text->value); + $this->assertEquals($text, $expanded->$text->description); + } + + /** + * Test that exported user preference is shortened when exceeds the limit. + * + * @dataProvider long_filename_provider + * @param string $longtext + * @param string $expected + * @param string $text + */ + public function test_export_user_preference_long_filename($longtext, $expected, $text) { + $this->resetAfterTest(); + + if (!array_key_exists('json', core_filetypes::get_types())) { + // Add json as mime type to avoid lose the extension when shortening filenames. + core_filetypes::add_type('json', 'application/json', 'archive', [], '', 'JSON file archive'); + } + $expectedpath = 'System/User preferences/'.$expected.'.json'; + + $context = \context_system::instance(); + $component = $longtext; + + $writer = $this->get_writer_instance() + ->set_context($context) + ->export_user_preference($component, $text, $text, $text); + + $fileroot = $this->fetch_exported_content($writer); + + $contextpath = $this->get_context_path($context, [get_string('userpreferences')], "{$component}.json"); + $this->assertEquals($expectedpath, $contextpath); + + $json = $fileroot->getChild($contextpath)->getContent(); + $this->assertRegExp("/$text.*$text.*$text/", $json); + + $expanded = json_decode($json); + $this->assertTrue(isset($expanded->$text)); + $this->assertEquals($text, $expanded->$text->value); + $this->assertEquals($text, $expanded->$text->description); + } + + /** + * Provider for long filenames. + * + * @return array + */ + public function long_filename_provider() { + return [ + 'More than 100 characters' => [ + 'Etiam sit amet dui vel leo blandit viverra. Proin viverra suscipit velit. Aenean efficitur suscipit nibh nec suscipit', + 'Etiam sit amet dui vel leo blandit viverra. Proin viverra suscipit velit. Aenean effici - 22f7a5030d', + 'value', + ], + ]; + } + /** * Get a fresh content writer. *