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

          Issue Links

            Activity

            Hide
            Ankit Agarwal added a comment -

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

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

            Hi Marina,

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

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

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

            Show
            Marina Glancy added a comment - Please don't integrate it. It is already part of filemanager changes in MDL-31901
            Hide
            Marina Glancy added a comment -
            Show
            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 Agarwal added a comment -

            In that case this issue can be closed.
            Thanks

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

            Does this still need to be backported to 2.2?

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

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

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

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

            Show
            Charles Fulton added a comment - Updated with link to 2.1 and 2.2 branches (it's not a clean cherry-pick).
            Hide
            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
            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
            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
            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
            Rajesh Taneja added a comment -

            Works Great
            Thanks for fixing this, Charles.

            FYI:
            Tested on 21 and 22.

            Show
            Rajesh Taneja added a comment - Works Great Thanks for fixing this, Charles. FYI: Tested on 21 and 22.
            Hide
            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
            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: