Uploaded image for project: 'Moodle'
  1. Moodle
  2. MDL-72254

file_save_draft_area_files should check for $itemid == false and throw an exception instead of wiping your files

    XMLWordPrintable

    Details

    • Type: Improvement
    • Status: Closed
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: 3.9.9, 3.10.6, 3.11.2, 4.0
    • Fix Version/s: 4.0
    • Component/s: Files API
    • Labels:

      Description

      We have just had a nasty incident where a bug in our code lead to thousands of files being deleted which should not have been.

      In some form-save code, there was insufficient error-handling, so we ended up calling file_save_draft_area_files with $itemid === false.

      That means $oldfiles gets set to all the files for that (contextid, component, filearea) and then at the end of the function, it wipes any of those $oldfiles which it did not use.

      There is no situation where you would want this to happen, and it was a very subtle bug to chase down, becuase no errors are logged. The file API is doing exactly what it was told.

      Therefore, I think that we should get file_save_draft_area_files to check for $itemid === false before calling get_area_files, and to throw a helpful coding exception if it is.

        Attachments

          Activity

            People

            Assignee:
            timhunt Tim Hunt
            Reporter:
            timhunt Tim Hunt
            Peer reviewer:
            Mark Johnson Mark Johnson
            Integrator:
            Eloy Lafuente (stronk7) Eloy Lafuente (stronk7)
            Tester:
            CiBoT CiBoT
            Participants:
            Component watchers:
            Matteo Scaramuccia, Andrew Lyons, Dongsheng Cai, Huong Nguyen, Jun Pataleta, Michael Hawkins, Shamim Rezaie, Simey Lameze
            Votes:
            3 Vote for this issue
            Watchers:
            4 Start watching this issue

              Dates

              Created:
              Updated:
              Resolved:
              Fix Release Date:
              13/Dec/21

                Time Tracking

                Estimated:
                Original Estimate - Not Specified
                Not Specified
                Remaining:
                Remaining Estimate - 0 minutes
                0m
                Logged:
                Time Spent - 10 minutes
                10m