Description of the problem :
When uploading a file that exceeds the upload_max_filesize of the PHP server, no error message is printed by the file_save_upload function.
The problem can be reproduced adding an image that exceeds the upload max size thanks to the Image module.

Location of the bug :
The problem is that the piece of code that handle the upload errors is not correctly placed in the file_save_upload() function. Here is how the code is built (Drupal 6.2) :

function file_save_upload(...) {
   ....
   if (isset($_FILES['files']) && $_FILES['files']['name'][$source] && is_uploaded_file($_FILES['files']['tmp_name'][$source])) {

      // Check for file upload errors and return FALSE if a low level system error occurred.
      switch ($_FILES['files']['error'][$source]) {
         case UPLOAD_ERR_OK:
            break;
         case UPLOAD_ERR_INI_SIZE:
         case UPLOAD_ERR_FORM_SIZE:
            print error message
            return 0;
         ....
         default:
            ....
            return 0;
       }
       Build the $file object
       return $file;
   }
   return 0;
}

The is_uploaded_file() function returns FALSE if the uploaded file exceeds the upload_max_filesize of the PHP server. Consequently, the error handling code is not evaluated and the file_save_upload() function returns 0 without printing any error message.
The solution i propose (see attached patch) is to rewrite it the following way :

function file_save_upload(...) {
   .... 
   // If no file was uploaded, return 0 (unexpected error)
   if (!isset($_FILES['files']) || !$_FILES['files']['name'][$source])
      return 0;

   // Check for file upload errors and return FALSE if a low level system error occurred.
   if (!is_uploaded_file($_FILES['files']['tmp_name'][$source])  || $_FILES['files']['error'][$source] != UPLOAD_ERR_OK) {
      switch ($_FILES['files']['error'][$source]) {
         case UPLOAD_ERR_OK:
            break;
         case UPLOAD_ERR_INI_SIZE:
         case UPLOAD_ERR_FORM_SIZE:
            print error message
            return 0;
         ....
         default:
            ....
            return 0;
       }
   }
   Build the $file object
   return $file;
}

System configuration :
OS : Ubuntu 7.10
Apache 2.2.4, PHP 5.2.3, Drupal 6.2, Image module 6.x-1.0-alpha1

CommentFileSizeAuthor
file.inc.patch7.02 KBStephaneC

Comments

mcdruid’s picture

Thanks for your notes on this - the same issue is present in drupal 5.7: http://drupal.org/node/255298 - I've submitted a patch for 5.x version of file.inc

drumm’s picture

Status: Needs work » Needs review

Marked http://drupal.org/node/255298 as a duplicate of this. http://drupal.org/files/issues/file.inc__8.patch is the 5.x patch uploaded by mcdruid.

drewish’s picture

Status: Needs review » Closed (duplicate)