Skip to content

Commit 3fe1059

Browse files
marinaglancyDamyon Wiese
authored and
Damyon Wiese
committed
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
1 parent f0ab42f commit 3fe1059

File tree

3 files changed

+15
-12
lines changed

3 files changed

+15
-12
lines changed

Diff for: repository/filepicker.php

+1-1
Original file line numberDiff line numberDiff line change
@@ -293,7 +293,7 @@
293293
// note that in this case user may not have permission to access the source file directly
294294
// so no file_browser/file_info can be used below
295295
if ($repo->has_moodle_files()) {
296-
$file = repository::get_moodle_file($fileurl);
296+
$file = repository::get_moodle_file($reference);
297297
if ($file && $file->is_external_file()) {
298298
$sourcefield = $file->get_source(); // remember the original source
299299
$record->source = $repo::build_source_field($sourcefield);

Diff for: repository/lib.php

+13-10
Original file line numberDiff line numberDiff line change
@@ -799,13 +799,14 @@ public static function draftfile_exists($itemid, $filepath, $filename) {
799799
}
800800

801801
/**
802-
* Parses the 'source' returned by moodle repositories and returns an instance of stored_file
802+
* Parses the moodle file reference and returns an instance of stored_file
803803
*
804-
* @param string $source
804+
* @param string $reference reference to the moodle internal file as retruned by
805+
* {@link repository::get_file_reference()} or {@link file_storage::pack_reference()}
805806
* @return stored_file|null
806807
*/
807-
public static function get_moodle_file($source) {
808-
$params = file_storage::unpack_reference($source, true);
808+
public static function get_moodle_file($reference) {
809+
$params = file_storage::unpack_reference($reference, true);
809810
$fs = get_file_storage();
810811
return $fs->get_file($params['contextid'], $params['component'], $params['filearea'],
811812
$params['itemid'], $params['filepath'], $params['filename']);
@@ -817,13 +818,14 @@ public static function get_moodle_file($source) {
817818
* This is checked when user tries to pick the file from repository to deal with
818819
* potential parameter substitutions is request
819820
*
820-
* @param string $source
821+
* @param string $source source of the file, returned by repository as 'source' and received back from user (not cleaned)
821822
* @return bool whether the file is accessible by current user
822823
*/
823824
public function file_is_accessible($source) {
824825
if ($this->has_moodle_files()) {
826+
$reference = $this->get_file_reference($source);
825827
try {
826-
$params = file_storage::unpack_reference($source, true);
828+
$params = file_storage::unpack_reference($reference, true);
827829
} catch (file_reference_exception $e) {
828830
return false;
829831
}
@@ -1401,12 +1403,13 @@ public function get_file_by_reference($reference) {
14011403
* again to another file area (also as a copy or as a reference), the value of
14021404
* files.source is copied.
14031405
*
1404-
* @param string $source the value that repository returned in listing as 'source'
1406+
* @param string $source source of the file, returned by repository as 'source' and received back from user (not cleaned)
14051407
* @return string|null
14061408
*/
14071409
public function get_file_source_info($source) {
14081410
if ($this->has_moodle_files()) {
1409-
return $this->get_reference_details($source, 0);
1411+
$reference = $this->get_file_reference($source);
1412+
return $this->get_reference_details($reference, 0);
14101413
}
14111414
return $source;
14121415
}
@@ -1687,11 +1690,11 @@ public static function display_instances_list($context, $typename = null) {
16871690
/**
16881691
* Prepare file reference information
16891692
*
1690-
* @param string $source
1693+
* @param string $source source of the file, returned by repository as 'source' and received back from user (not cleaned)
16911694
* @return string file referece
16921695
*/
16931696
public function get_file_reference($source) {
1694-
if ($this->has_moodle_files() && ($this->supported_returntypes() & FILE_REFERENCE)) {
1697+
if ($source && $this->has_moodle_files()) {
16951698
$params = file_storage::unpack_reference($source);
16961699
if (!is_array($params)) {
16971700
throw new repository_exception('invalidparams', 'repository');

Diff for: repository/repository_ajax.php

+1-1
Original file line numberDiff line numberDiff line change
@@ -238,7 +238,7 @@
238238
// note that in this case user may not have permission to access the source file directly
239239
// so no file_browser/file_info can be used below
240240
if ($repo->has_moodle_files()) {
241-
$file = repository::get_moodle_file($source);
241+
$file = repository::get_moodle_file($reference);
242242
if ($file && $file->is_external_file()) {
243243
$sourcefield = $file->get_source(); // remember the original source
244244
$record->source = $repo::build_source_field($sourcefield);

0 commit comments

Comments
 (0)