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:
    • Rank:
      39925

      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).

        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: