From 836f8eb62e9a4551d0f53fd959054c3d9fbab8f1 Mon Sep 17 00:00:00 2001 From: Shamim Rezaie Date: Fri, 11 May 2018 17:19:12 +1000 Subject: [PATCH 1/4] MDL-62251 backup: Fix replace_tempdir() bug under Windows rename() fails under Windows if the destination file/directory exists. I modified the code to only call $this->get_workdir_path() once as that function creates the directory if doesn't exist. And we don't want that considering the behaviour of rename on Windows. --- backup/converter/convertlib.php | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/backup/converter/convertlib.php b/backup/converter/convertlib.php index ef37f5ea3ca..efcd6230658 100644 --- a/backup/converter/convertlib.php +++ b/backup/converter/convertlib.php @@ -239,15 +239,17 @@ abstract class base_converter implements loggable { protected function replace_tempdir() { global $CFG; + $tempdir = $this->get_tempdir_path(); + if (empty($CFG->keeptempdirectoriesonbackup)) { - fulldelete($this->get_tempdir_path()); + fulldelete($tempdir); } else { - if (!rename($this->get_tempdir_path(), $this->get_tempdir_path() . '_' . $this->get_name() . '_' . $this->id . '_source')) { + if (!rename($tempdir, $tempdir . '_' . $this->get_name() . '_' . $this->id . '_source')) { throw new convert_exception('failed_rename_source_tempdir'); } } - if (!rename($this->get_workdir_path(), $this->get_tempdir_path())) { + if (!rename($this->get_workdir_path(), $tempdir)) { throw new convert_exception('failed_move_converted_into_place'); } } From b561a9a3c2b93ec398cbb6eddd14e47fa060b79e Mon Sep 17 00:00:00 2001 From: Shamim Rezaie Date: Fri, 11 May 2018 17:27:49 +1000 Subject: [PATCH 2/4] MDL-62251 Privacy: Fix get_path() and get_full_path() bug in Windows --- .../classes/local/request/moodle_content_writer.php | 8 ++++++-- privacy/tests/moodle_content_writer_test.php | 10 ++++++++-- 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/privacy/classes/local/request/moodle_content_writer.php b/privacy/classes/local/request/moodle_content_writer.php index c212e271df8..2c642512bad 100644 --- a/privacy/classes/local/request/moodle_content_writer.php +++ b/privacy/classes/local/request/moodle_content_writer.php @@ -272,7 +272,9 @@ class moodle_content_writer implements content_writer { // Join the directory together with the name. $filepath = implode(DIRECTORY_SEPARATOR, $path) . DIRECTORY_SEPARATOR . $name; - return preg_replace('@' . DIRECTORY_SEPARATOR . '+@', DIRECTORY_SEPARATOR, $filepath); + // To use backslash, it must be doubled ("\\\\" PHP string). + $separator = str_replace('\\', '\\\\', DIRECTORY_SEPARATOR); + return preg_replace('@(' . $separator . '|/)+@', $separator, $filepath); } /** @@ -291,7 +293,9 @@ class moodle_content_writer implements content_writer { // Join the directory together with the name. $filepath = implode(DIRECTORY_SEPARATOR, $path); - return preg_replace('@' . DIRECTORY_SEPARATOR . '+@', DIRECTORY_SEPARATOR, $filepath); + // To use backslash, it must be doubled ("\\\\" PHP string). + $separator = str_replace('\\', '\\\\', DIRECTORY_SEPARATOR); + return preg_replace('@(' . $separator . '|/)+@', $separator, $filepath); } /** diff --git a/privacy/tests/moodle_content_writer_test.php b/privacy/tests/moodle_content_writer_test.php index 7e20f64879b..66e6759b8f1 100644 --- a/privacy/tests/moodle_content_writer_test.php +++ b/privacy/tests/moodle_content_writer_test.php @@ -1162,12 +1162,18 @@ class moodle_content_writer_test extends advanced_testcase { if (null === $subcontext) { $rcm = $rc->getMethod('get_context_path'); $rcm->setAccessible(true); - return $rcm->invoke($writer); + $path = $rcm->invoke($writer); } else { $rcm = $rc->getMethod('get_path'); $rcm->setAccessible(true); - return $rcm->invoke($writer, $subcontext, $name); + $path = $rcm->invoke($writer, $subcontext, $name); } + + // PHPUnit uses mikey179/vfsStream which is a stream wrapper for a virtual file system that uses '/' + // as the directory separator. + $path = str_replace(DIRECTORY_SEPARATOR, '/', $path); + + return $path; } /** From 6d882faecfa18160c418a043fb8d7b34270d2faf Mon Sep 17 00:00:00 2001 From: Shamim Rezaie Date: Fri, 11 May 2018 17:31:23 +1000 Subject: [PATCH 3/4] MDL-62251 Privacy: Url path separator should be platform independant It should always be forward slash. --- .../local/request/moodle_content_writer.php | 21 ++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/privacy/classes/local/request/moodle_content_writer.php b/privacy/classes/local/request/moodle_content_writer.php index 2c642512bad..34dacba0945 100644 --- a/privacy/classes/local/request/moodle_content_writer.php +++ b/privacy/classes/local/request/moodle_content_writer.php @@ -162,7 +162,7 @@ class moodle_content_writer implements content_writer { * @return string The processed string */ public function rewrite_pluginfile_urls(array $subcontext, $component, $filearea, $itemid, $text) : string { - return str_replace('@@PLUGINFILE@@/', $this->get_files_target_path($component, $filearea, $itemid).'/', $text); + return str_replace('@@PLUGINFILE@@/', $this->get_files_target_url($component, $filearea, $itemid).'/', $text); } /** @@ -318,6 +318,25 @@ class moodle_content_writer implements content_writer { return implode(DIRECTORY_SEPARATOR, $parts); } + /** + * Get a relative url to the directory of the exported files within a subcontext. + * + * @param string $component The name of the component that the files belong to. + * @param string $filearea The filearea within that component. + * @param string $itemid Which item those files belong to. + * @return string The url + */ + protected function get_files_target_url($component, $filearea, $itemid) : string { + // We do not need to include the component because we organise things by context. + $parts = ['_files', $filearea]; + + if (!empty($itemid)) { + $parts[] = $itemid; + } + + return implode('/', $parts); + } + /** * Write the data to the specified path. * From 5604f5977f273901cc1cc8ec1d07e5be33929ce4 Mon Sep 17 00:00:00 2001 From: Shamim Rezaie Date: Fri, 11 May 2018 17:33:08 +1000 Subject: [PATCH 4/4] MDL-62251 Privacy: Fix dir separator in export_file() --- privacy/classes/tests/request/content_writer.php | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/privacy/classes/tests/request/content_writer.php b/privacy/classes/tests/request/content_writer.php index 3bd7e9da749..f91025a4e67 100644 --- a/privacy/classes/tests/request/content_writer.php +++ b/privacy/classes/tests/request/content_writer.php @@ -396,7 +396,10 @@ class content_writer implements \core_privacy\local\request\content_writer { */ public function export_file(array $subcontext, \stored_file $file) : \core_privacy\local\request\content_writer { if (!$file->is_directory()) { - $filepath = explode(DIRECTORY_SEPARATOR, $file->get_filepath()); + $filepath = $file->get_filepath(); + // Directory separator in the stored_file class should always be '/'. The following line is just a fail safe. + $filepath = str_replace(DIRECTORY_SEPARATOR, '/', $filepath); + $filepath = explode('/', $filepath); $filepath[] = $file->get_filename(); $filepath = array_filter($filepath); $filepath = implode('/', $filepath);