Mahara

External file system image infinite loop

Bug #1776364 reported by Kenneth Hendricks
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Mahara
Fix Released
High
Unassigned
Mahara 18.10.0
18.04
Fix Released
High
Unassigned
Mahara 18.04.3
18.10
Fix Released
High
Unassigned
Mahara 18.10.0

Bug Description

When the objectfs plugin is being used as an external file system, an image is uploaded and that image is no longer locally available the following infinite loop occurs:

mahara_1 | PHP message: PHP 34291. module_objectfs\object_file_system->is_file_readable_locally() /siteroot/htdocs/module/objectfs/classes/object_file_system.php:141
mahara_1 | PHP message: PHP 34292. ArtefactTypeImage->get_local_path() /siteroot/htdocs/module/objectfs/classes/object_file_system.php:122
mahara_1 | PHP message: PHP 34293. get_dataroot_image_path() /siteroot/htdocs/artefact/file/lib.php:2391
mahara_1 | PHP message: PHP 34294. ArtefactTypeFile->ensure_local() /siteroot/htdocs/lib/file.php:542
mahara_1 | PHP message: PHP 34295. module_objectfs\mahara_external_filesystem->ensure_local() /siteroot/htdocs/artefact/file/lib.php:1147
mahara_1 | PHP message: PHP 34296. module_objectfs\mahara_external_filesystem->get_file_location_status() /siteroot/htdocs/module/objectfs/classes/mahara_external_filesystem.php:64
mahara_1 | PHP message: PHP 34297. module_objectfs\object_file_system->get_object_location() /siteroot/htdocs/module/objectfs/classes/mahara_external_filesystem.php:87
mahara_1 | PHP message: PHP 34298. module_objectfs\object_file_system->is_file_readable_locally() /siteroot/htdocs/module/objectfs/classes/object_file_system.php:141

It's my opinion this is not an issue with the plugin itself, but the core hooks. It is expected that a FS plugin will call ArtefactTypeImage->get_local_path within its mahara_external_filesystem->ensure_local.

Reproduction steps (reproduced with 1804):

1. Fresh install mahara
2. Install https://github.com/catalyst/mahara-module_objectfs and https://github.com/catalyst/mahara-module_aws
3. Configure the plugin with test s3 crendentials.
4. Set:
    Delete local objects = true
    Minimum size threshold = 0
    Minimum age = 0
    Consistancy delay = 0

5. Put this line in config.php:

$cfg->externalfilesystem = array(
   "includefilepath" => "module/objectfs/classes/s3_file_system.php",
   "class" => "module_objectfs\\s3_file_system"
);

6. Goto /artefact/file/index.php

7. Upload an image

8. php htdocs/module/objectfs/cli/run_pusher.php

9. php htdocs/module/objectfs/cli/run_deleter.php // The file will now be in s3 only

10. Refresh /artefact/file/index.php and view error

Revision history for this message
Mahara Bot (dev-mahara) wrote : A patch has been submitted for review

Patch for "master" branch: https://reviews.mahara.org/8950

Revision history for this message
Kenneth Hendricks (kennethhendricks) wrote :

Hi all,

Added a possible patch to fix this. It works by adding a generateifpossible flag to get_local_path which is used within get_dataroot_image_path to gate ensure_local. External FS's can then call get_local_path with this flag set to false to avoid calling any of the generating code. Additionally get_local_path will return the local FS path even if it is not readable in this case.

The ideal solution would probably involve decoupling the actual path retrieval and image generation in get_dataroot_image_path, however as a non-mahara developer I was not comfortable with the code footprint that would be required to do so.

Thanks,
KC

Revision history for this message
Kenneth Hendricks (kennethhendricks) wrote :

Companion update to objectfs to accompany this for testing:

https://github.com/catalyst/mahara-module_objectfs/tree/issue-8

Revision history for this message
Mahara Bot (dev-mahara) wrote : A change has been merged

Reviewed: https://reviews.mahara.org/8950
Committed: https://git.mahara.org/mahara/mahara/commit/63d04fd120c2d848e986542ee2368798373cefad
Submitter: Robert Lyon (<email address hidden>)
Branch: master

commit 63d04fd120c2d848e986542ee2368798373cefad
Author: Kenneth Hendricks <email address hidden>
Date: Tue Jun 12 12:27:42 2018 +1000

Bug 1776364: Add generateifpossible flag to get_local_path

This flag will set whether the function should
attempt to create the file if it doesn't exist.

E.g. image resizes.

This allows an external file system to call
get_local_path with this flag set to false to
skip the processing. This allows an external
file system to avoid an infinate loop when calling
get_local_path to check local file availibility.

Sponsored by Central Queensland University

behatnotneeded

Change-Id: Iaade0b06fcc2cd1755467231e0bafea6b90a99ec

Robert Lyon (robertl-9)
Changed in mahara:
status: New → Fix Committed
importance: Undecided → High
milestone: none → 18.10.0
Revision history for this message
Mahara Bot (dev-mahara) wrote : A patch has been submitted for review

Patch for "18.04_STABLE" branch: https://reviews.mahara.org/8961

Revision history for this message
Mahara Bot (dev-mahara) wrote : A change has been merged

Reviewed: https://reviews.mahara.org/8961
Committed: https://git.mahara.org/mahara/mahara/commit/ebc59442e8119263b56b1a1295a6295743de3a9d
Submitter: Robert Lyon (<email address hidden>)
Branch: 18.04_STABLE

commit ebc59442e8119263b56b1a1295a6295743de3a9d
Author: Kenneth Hendricks <email address hidden>
Date: Tue Jun 12 12:27:42 2018 +1000

Bug 1776364: Add generateifpossible flag to get_local_path

This flag will set whether the function should
attempt to create the file if it doesn't exist.

E.g. image resizes.

This allows an external file system to call
get_local_path with this flag set to false to
skip the processing. This allows an external
file system to avoid an infinate loop when calling
get_local_path to check local file availibility.

Sponsored by Central Queensland University

behatnotneeded

Change-Id: Iaade0b06fcc2cd1755467231e0bafea6b90a99ec
(cherry picked from commit 63d04fd120c2d848e986542ee2368798373cefad)

To post a comment you must log in.
This report contains Public information  
Everyone can see this information.
Subscribing...

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.