Merge branch 'MDL-62853' of https://github.com/paulholden/moodle

privacy/classes/local/request/moodle_content_writer.php

@@ -139,11 +139,8 @@ class moodle_content_writer implements content_writer {
      * @return  content_writer
      */
     public function export_related_data(array $subcontext, $name, $data) : content_writer {
-        $path = $this->get_path($subcontext, "{$name}.json");
+        return $this->export_custom_file($subcontext, "{$name}.json",
-
+            json_encode($data, JSON_UNESCAPED_UNICODE | JSON_PRETTY_PRINT));
-        $this->write_data($path, json_encode($data, JSON_UNESCAPED_UNICODE | JSON_PRETTY_PRINT));
-
-        return $this;
     }
 
     /**
@@ -289,6 +286,7 @@ class moodle_content_writer implements content_writer {
         // This weird code is to look for a subcontext that contains a number and append an '_' to the front.
         // This is because there seems to be some weird problem with array_merge_recursive used in finalise_content().
         $subcontext = array_map(function($data) {
+            $data = clean_param($data, PARAM_PATH);
             if (stripos($data, DIRECTORY_SEPARATOR) !== false) {
                 $newpath = explode(DIRECTORY_SEPARATOR, $data);
                 $newpath = array_map(function($value) {

privacy/tests/moodle_content_writer_test.php

@@ -963,6 +963,34 @@ class moodle_content_writer_test extends advanced_testcase {
         $this->assertEquals($data, $expanded);
     }
 
+    /**
+     * Test that exported related data name is properly cleaned
+     *
+     * @covers ::export_related_data
+     */
+    public function test_export_related_data_clean_name() {
+        $context = \context_system::instance();
+        $subcontext = [];
+        $data = (object) ['foo' => 'bar'];
+
+        $name = 'Bad/chars:>';
+
+        $writer = $this->get_writer_instance()
+            ->set_context($context)
+            ->export_related_data($subcontext, $name, $data);
+
+        $nameclean = clean_param($name, PARAM_FILE);
+
+        $contextpath = $this->get_context_path($context, $subcontext, "{$nameclean}.json");
+        $expectedpath = "System _.{$context->id}/Badchars.json";
+        $this->assertEquals($expectedpath, $contextpath);
+
+        $fileroot = $this->fetch_exported_content($writer);
+        $json = $fileroot->getChild($contextpath)->getContent();
+
+        $this->assertEquals($data, json_decode($json));
+    }
+
     /**
      * Test that exported user preference is human readable.
      *
@@ -1002,6 +1030,30 @@ class moodle_content_writer_test extends advanced_testcase {
         ];
     }
 
+    /**
+     * Test that exported data subcontext is properly cleaned
+     *
+     * @covers ::export_data
+     */
+    public function test_export_data_clean_subcontext() {
+        $context = \context_system::instance();
+        $subcontext = ['Something/weird', 'More/bad:>', 'Bad&chars:>'];
+        $data = (object) ['foo' => 'bar'];
+
+        $writer = $this->get_writer_instance()
+            ->set_context($context)
+            ->export_data($subcontext, $data);
+
+        $contextpath = $this->get_context_path($context, $subcontext, 'data.json');
+        $expectedpath = "System _.{$context->id}/Something/weird/More/bad/Badchars/data.json";
+        $this->assertEquals($expectedpath, $contextpath);
+
+        $fileroot = $this->fetch_exported_content($writer);
+        $json = $fileroot->getChild($contextpath)->getContent();
+
+        $this->assertEquals($data, json_decode($json));
+    }
+
     /**
      * Test that exported data is shortened when exceeds the limit.
      *