Uploaded image for project: 'eZ Publish / Platform'
  1. eZ Publish / Platform
  2. EZP-15083

No warning is given when uploading file and size exeeds post_max_size [PATCH]

    Details

      Description

      Hi

      The ezbinaryfile has support for detecting if file is bigger than upload_max_file_size. However, ezbinaryfile doesn't detect if file size exceeds post_max_size.
      This is not possible to detect by a datatype either since all post variables are removed in such case. It therefore needs to be detected elsewhere (like in content/edit)

      Here is a patch which does the trick.

      Regarding my comment:

      if( ( (int) $_SERVER['CONTENT_LENGTH'] > $postMaxSize ) &&  // This is not 100% accurate as $_SERVER['CONTENT_LENGTH'] doesn't only count post data but also other things
      

      In PHP's source code you have this (main/rfc1867.c):

      if (SG(request_info).content_length > SG(post_max_size)) {
                      sapi_module.sapi_error(E_WARNING, "POST Content-Length of %ld bytes exceeds the limit of %ld bytes", SG(request_info).content_length, SG(post_max_size));
                      return;
              }
      

      I guess SG(request_info).content_length is the size of the whole request, not only the post data. But I am not sure...Anyway, since we also check if we indeed has no POST variables I think the detection in my patch is quite OK.

      1. 15083_detect_post_max_size.patch
        2 kB
        Vidar Langseid
      2. 15083_detect_post_max_size-show_size.patch
        2 kB
        (inactive) Gunnstein Lye
      3. attribute_edit.php.diff
        4 kB
        (inactive) Gunnstein Lye

        Issue Links

          Activity

          Hide
          Vidar Langseid added a comment -

          15083_detect_post_max_size.patch 15083_detect_post_max_size.patch

          Show
          Vidar Langseid added a comment - 15083_detect_post_max_size.patch 15083_detect_post_max_size.patch
          Hide
          (inactive) Gunnstein Lye added a comment -

          Attached a version that shows the user the allowed max size. Should we do this?15083_detect_post_max_size-show_size.patch

          Show
          (inactive) Gunnstein Lye added a comment - Attached a version that shows the user the allowed max size. Should we do this? 15083_detect_post_max_size-show_size.patch
          Hide
          Vidar Langseid added a comment -

          Fine by me.

          It might be considered to be a security problem to reveal this information ? Anyway, it is quite easy to figure out this number if you really want, so I don't think it is a problem.

          Show
          Vidar Langseid added a comment - Fine by me. It might be considered to be a security problem to reveal this information ? Anyway, it is quite easy to figure out this number if you really want, so I don't think it is a problem.
          Hide
          (inactive) Gunnstein Lye added a comment -

          Fixed in
          trunk (4.2.0Alpha1) rev. 23730
          stable/4.1 (4.1.4) rev. 23731
          stable/4.0 (4.0.7) rev. 23732

          Show
          (inactive) Gunnstein Lye added a comment - Fixed in trunk (4.2.0Alpha1) rev. 23730 stable/4.1 (4.1.4) rev. 23731 stable/4.0 (4.0.7) rev. 23732
          Hide
          Vidar Langseid added a comment -

          Unfortunately, I found a bug in my patch suggestion.
          The variable $_SERVER['REQUEST_METHOD'] is only set if we indeed are dealing with a POST request. If you send a GET request to content/edit you'll then get a PHP notification ("Undefined index").
          So all code in the patch should be placed inside a "if" :

          if( $_SERVER['REQUEST_METHOD'] === 'POST' )
          {
              // Need to detect if post_max_size has been reached. If so, all post variables are gone....
             ...
          }
          

          Show
          Vidar Langseid added a comment - Unfortunately, I found a bug in my patch suggestion. The variable $_SERVER ['REQUEST_METHOD'] is only set if we indeed are dealing with a POST request. If you send a GET request to content/edit you'll then get a PHP notification ("Undefined index"). So all code in the patch should be placed inside a "if" : if( $_SERVER['REQUEST_METHOD'] === 'POST' ) { // Need to detect if post_max_size has been reached. If so, all post variables are gone.... ... }
          Hide
          (inactive) Gunnstein Lye added a comment -

          In reply to comment #048796
          Like this (attachment)? It assumes that we do not need to care about sizes during a GET, since you can only upload files with POST.attribute_edit.php.diff

          Show
          (inactive) Gunnstein Lye added a comment - In reply to comment #048796 Like this (attachment)? It assumes that we do not need to care about sizes during a GET, since you can only upload files with POST. attribute_edit.php.diff
          Hide
          (inactive) Gunnstein Lye added a comment -

          Fixed in
          trunk (4.2.0Alpha1) rev. 23784
          stable/4.1 (4.1.4) rev. 23785
          stable/4.0 (4.0.7) rev. 23786

          Show
          (inactive) Gunnstein Lye added a comment - Fixed in trunk (4.2.0Alpha1) rev. 23784 stable/4.1 (4.1.4) rev. 23785 stable/4.0 (4.0.7) rev. 23786
          Hide
          Vidar Langseid added a comment -
          Show
          Vidar Langseid added a comment - In reply to comment #048797 Yes, like that
          Hide
          ezrobot added a comment -

          This issue has been automatically closed due to the lack of activity over a long period of time. It is very likely that it is obsolete, but if you think it is still valid, do not hesitate to reopen it and mention why.

          Show
          ezrobot added a comment - This issue has been automatically closed due to the lack of activity over a long period of time. It is very likely that it is obsolete, but if you think it is still valid, do not hesitate to reopen it and mention why.

            People

            • Assignee:
              (inactive) Gunnstein Lye
              Reporter:
              Vidar Langseid
            • Votes:
              0 Vote for this issue
              Watchers:
              0 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: