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

Course file upload limit is ignored when maxbytes is set to server limit

    Details

    • Testing Instructions:
      Hide

      1. Set the server maximum uploaded file size to some numeric value (A).
      2. Set the course maximum uploaded file size to some numeric value less than A (B).
      3. Attempt to upload a file with size C, where C is larger than B but smaller than A. Verify that it fails.
      4. Attempt to upload a file with size D, where D is smaller than A and B. Verify that it succeeds.
      5. Set the server maximum uploaded file size to "Server limit."
      6. Repeat steps 2 through 4.

      Show
      1. Set the server maximum uploaded file size to some numeric value (A). 2. Set the course maximum uploaded file size to some numeric value less than A (B). 3. Attempt to upload a file with size C, where C is larger than B but smaller than A. Verify that it fails. 4. Attempt to upload a file with size D, where D is smaller than A and B. Verify that it succeeds. 5. Set the server maximum uploaded file size to "Server limit." 6. Repeat steps 2 through 4.
    • Affected Branches:
      MOODLE_21_STABLE, MOODLE_22_STABLE, MOODLE_23_STABLE
    • Fixed Branches:
      MOODLE_21_STABLE, MOODLE_22_STABLE
    • Pull from Repository:

      Description

      We've noticed an inconsistency with how Server and Course file upload limits interact when one is less than the other. If Server limit (maxbytes) is set to 10MB, and the Course limit to 5MB, then a 7MB file is rejected (and the message clearly mentions a form restriction and not any server limitation).

      However, let's say that maxbytes is set to "Server limit" (0), and the Course limit is still 5MB. A 7MB file will upload without any problems, ignoring the course limit.

      Observed in 2.2.1 and current master (2.3).

        Gliffy Diagrams

          Attachments

            Issue Links

              Activity

              Hide
              ankit_frenz Ankit Agarwal added a comment -

              Hi charles,
              Changes look good.
              +1 to integrate
              Thanks

              Show
              ankit_frenz Ankit Agarwal added a comment - Hi charles, Changes look good. +1 to integrate Thanks
              Hide
              poltawski Dan Poltawski added a comment -

              Hi Marina,

              I've just added you here in case this impacts anything you are working on?

              Show
              poltawski Dan Poltawski added a comment - Hi Marina, I've just added you here in case this impacts anything you are working on?
              Hide
              marina Marina Glancy added a comment -

              Please don't integrate it. It is already part of filemanager changes in MDL-31901

              Show
              marina Marina Glancy added a comment - Please don't integrate it. It is already part of filemanager changes in MDL-31901
              Hide
              marina Marina Glancy added a comment -
              Show
              marina Marina Glancy added a comment - https://github.com/marinaglancy/moodle/compare/master...wip-MDL-31901-master#L7R351 (file lib/form/filemanager.php, line 341)
              Hide
              ankit_frenz Ankit Agarwal added a comment -

              In that case this issue can be closed.
              Thanks

              Show
              ankit_frenz Ankit Agarwal added a comment - In that case this issue can be closed. Thanks
              Hide
              poltawski Dan Poltawski added a comment -

              Does this still need to be backported to 2.2?

              Show
              poltawski Dan Poltawski added a comment - Does this still need to be backported to 2.2?
              Hide
              ankit_frenz Ankit Agarwal added a comment -

              oh yes completely forgot about 2.2
              +1 for the backport.
              Thanks

              Show
              ankit_frenz Ankit Agarwal added a comment - oh yes completely forgot about 2.2 +1 for the backport. Thanks
              Hide
              cfulton Charles Fulton added a comment -

              Updated with link to 2.1 and 2.2 branches (it's not a clean cherry-pick).

              Show
              cfulton Charles Fulton added a comment - Updated with link to 2.1 and 2.2 branches (it's not a clean cherry-pick).
              Hide
              stronk7 Eloy Lafuente (stronk7) added a comment -

              The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week.

              TIA and ciao

              Show
              stronk7 Eloy Lafuente (stronk7) added a comment - The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week. TIA and ciao
              Hide
              nebgor Aparup Banerjee added a comment -

              integrated into 22 and 21 only, not in master.

              note: keeping 'affects version of 2.3' since it does affect 2.3 even if fix is going in with the patch @ MDL-31901.

              Show
              nebgor Aparup Banerjee added a comment - integrated into 22 and 21 only, not in master. note: keeping 'affects version of 2.3' since it does affect 2.3 even if fix is going in with the patch @ MDL-31901 .
              Hide
              rajeshtaneja Rajesh Taneja added a comment -

              Works Great
              Thanks for fixing this, Charles.

              FYI:
              Tested on 21 and 22.

              Show
              rajeshtaneja Rajesh Taneja added a comment - Works Great Thanks for fixing this, Charles. FYI: Tested on 21 and 22.
              Hide
              stronk7 Eloy Lafuente (stronk7) added a comment -

              U P S T R E A M I Z E D !

              Many thanks for the hard work, closing this as fixed.

              Ciao

              Show
              stronk7 Eloy Lafuente (stronk7) added a comment - U P S T R E A M I Z E D ! Many thanks for the hard work, closing this as fixed. Ciao

                People

                • Votes:
                  0 Vote for this issue
                  Watchers:
                  3 Start watching this issue

                  Dates

                  • Created:
                    Updated:
                    Resolved:
                    Fix Release Date:
                    9/Jul/12