From cb2b42aed8d9ce3c9840ad825f2e0e7e81bfad91 Mon Sep 17 00:00:00 2001
From: Marina Glancy <marina@moodle.com>
Date: Wed, 25 Jun 2014 14:40:27 +0800
Subject: [PATCH] MDL-45616 repositories: more clearly distinguish when we use
 source and when reference

Function repository::get_moodle_file() should always be called on packed reference and not on the source received from user.
Also added phpdocs to some other methods that were confusing source and reference
---
 repository/filepicker.php      |  2 +-
 repository/lib.php             | 23 +++++++++++++----------
 repository/repository_ajax.php |  2 +-
 3 files changed, 15 insertions(+), 12 deletions(-)

diff --git a/repository/filepicker.php b/repository/filepicker.php
index bd19ef7f73b..20d5b872bb6 100644
--- a/repository/filepicker.php
+++ b/repository/filepicker.php
@@ -293,7 +293,7 @@ case 'download':
     // note that in this case user may not have permission to access the source file directly
     // so no file_browser/file_info can be used below
     if ($repo->has_moodle_files()) {
-        $file = repository::get_moodle_file($fileurl);
+        $file = repository::get_moodle_file($reference);
         if ($file && $file->is_external_file()) {
             $sourcefield = $file->get_source(); // remember the original source
             $record->source = $repo::build_source_field($sourcefield);
diff --git a/repository/lib.php b/repository/lib.php
index e9781e20bcc..3bec676b00e 100644
--- a/repository/lib.php
+++ b/repository/lib.php
@@ -717,13 +717,14 @@ abstract class repository {
     }
 
     /**
-     * Parses the 'source' returned by moodle repositories and returns an instance of stored_file
+     * Parses the moodle file reference and returns an instance of stored_file
      *
-     * @param string $source
+     * @param string $reference reference to the moodle internal file as retruned by
+     *        {@link repository::get_file_reference()} or {@link file_storage::pack_reference()}
      * @return stored_file|null
      */
-    public static function get_moodle_file($source) {
-        $params = file_storage::unpack_reference($source, true);
+    public static function get_moodle_file($reference) {
+        $params = file_storage::unpack_reference($reference, true);
         $fs = get_file_storage();
         return $fs->get_file($params['contextid'], $params['component'], $params['filearea'],
                     $params['itemid'], $params['filepath'], $params['filename']);
@@ -735,13 +736,14 @@ abstract class repository {
      * This is checked when user tries to pick the file from repository to deal with
      * potential parameter substitutions is request
      *
-     * @param string $source
+     * @param string $source source of the file, returned by repository as 'source' and received back from user (not cleaned)
      * @return bool whether the file is accessible by current user
      */
     public function file_is_accessible($source) {
         if ($this->has_moodle_files()) {
+            $reference = $this->get_file_reference($source);
             try {
-                $params = file_storage::unpack_reference($source, true);
+                $params = file_storage::unpack_reference($reference, true);
             } catch (file_reference_exception $e) {
                 return false;
             }
@@ -1336,12 +1338,13 @@ abstract class repository {
      * again to another file area (also as a copy or as a reference), the value of
      * files.source is copied.
      *
-     * @param string $source the value that repository returned in listing as 'source'
+     * @param string $source source of the file, returned by repository as 'source' and received back from user (not cleaned)
      * @return string|null
      */
     public function get_file_source_info($source) {
         if ($this->has_moodle_files()) {
-            return $this->get_reference_details($source, 0);
+            $reference = $this->get_file_reference($source);
+            return $this->get_reference_details($reference, 0);
         }
         return $source;
     }
@@ -1621,11 +1624,11 @@ abstract class repository {
     /**
      * Prepare file reference information
      *
-     * @param string $source
+     * @param string $source source of the file, returned by repository as 'source' and received back from user (not cleaned)
      * @return string file referece
      */
     public function get_file_reference($source) {
-        if ($this->has_moodle_files() && ($this->supported_returntypes() & FILE_REFERENCE)) {
+        if ($source && $this->has_moodle_files()) {
             $params = file_storage::unpack_reference($source);
             if (!is_array($params)) {
                 throw new repository_exception('invalidparams', 'repository');
diff --git a/repository/repository_ajax.php b/repository/repository_ajax.php
index fa8f488523f..8b5fe04ad7a 100644
--- a/repository/repository_ajax.php
+++ b/repository/repository_ajax.php
@@ -239,7 +239,7 @@ switch ($action) {
             // note that in this case user may not have permission to access the source file directly
             // so no file_browser/file_info can be used below
             if ($repo->has_moodle_files()) {
-                $file = repository::get_moodle_file($source);
+                $file = repository::get_moodle_file($reference);
                 if ($file && $file->is_external_file()) {
                     $sourcefield = $file->get_source(); // remember the original source
                     $record->source = $repo::build_source_field($sourcefield);