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

    • Improvement
    • Status: Closed
    • Minor
    • Resolution: Fixed
    • 3.9.9, 3.10.6, 3.11.2, 4.0
    • 4.0
    • Files API

    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

            timhunt Tim Hunt
            timhunt Tim Hunt
            Mark Johnson Mark Johnson
            Eloy Lafuente (stronk7) Eloy Lafuente (stronk7)
            CiBoT CiBoT
            Matteo Scaramuccia, Andrew Lyons, Huong Nguyen, Jun Pataleta, Michael Hawkins, Shamim Rezaie, Simey Lameze, Stevani Andolo
            Votes:
            3 Vote for this issue
            Watchers:
            4 Start watching this issue

            Dates

              Created:
              Updated:
              Resolved:
              19/Apr/22

              Time Tracking

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