- Patch #115267 by drewish, dopry et al: simplified file uploads code,...

- Patch #115267 by drewish, dopry et al: simplified file uploads code, improved file API, centralized file validation, implemented quotas and fixed file previews.

CHANGELOG.txt

@@ -48,6 +48,9 @@ Drupal 6.0, xxxx-xx-xx (development version)
     * Tags are now automatically closed at the end of the teaser.
 - Performance:
     * Made it easier to conditionally load include files.
+- File handling improvements:
+    * Entries in the files table are now keyed to a user, and not a node.
+    * Added re-usable validation functions to check for uploaded file sizes, extensions, and image resolution.
 
 Drupal 5.0, 2007-01-15
 ----------------------

includes/bootstrap.inc

@@ -1053,7 +1053,7 @@ function language_list($field = 'language', $reset = FALSE) {
 
 /**
  * Default language used on the site
- * 
+ *
  * @param $property
  *   Optional property of the language object to return
  */

includes/locale.inc

@@ -612,7 +612,7 @@ function locale_translate_import_form() {
  */
 function locale_translate_import_form_submit($form, &$form_state, $form_values) {
   // Ensure we have the file uploaded
-  if ($file = file_check_upload('file')) {
+  if ($file = file_save_upload('file')) {
 
     // Add language, if not yet supported
     $languages = language_list('language', TRUE);

modules/aggregator/aggregator.module

@@ -1080,7 +1080,7 @@ function aggregator_page_category() {
 
   drupal_add_feed(url('aggregator/rss/'. arg(2)), variable_get('site_name', 'Drupal') .' '. t('aggregator - @title', array('@title' => $category->title)));
 
-  return _aggregator_page_list('SELECT i.*, f.title AS ftitle, f.link AS flink FROM {aggregator_category_item} c LEFT JOIN {aggregator_item} i ON c.iid = i.iid LEFT JOIN {aggregator_feed} f ON i.fid = f.fid WHERE cid = '. $category->cid .' ORDER BY timestamp DESC, iid DESC', arg(3));
+  return _aggregator_page_list('SELECT i.*, f.title AS ftitle, f.link AS flink FROM {aggregator_category_item} c LEFT JOIN {aggregator_item} i ON c.iid = i.iid LEFT JOIN {aggregator_feed} f ON i.fid = f.fid WHERE cid = '. $category->cid .' ORDER BY timestamp DESC, i.iid DESC', arg(3));
 }
 
 function aggregator_page_list($sql, $header, $categorize) {
@@ -1223,7 +1223,7 @@ function aggregator_page_rss() {
     $category = db_fetch_object(db_query('SELECT cid, title FROM {aggregator_category} WHERE cid = %d', arg(2)));
     $url = '/categories/'. $category->cid;
     $title = ' '. t('in category') .' '. $category->title;
-    $sql = 'SELECT i.*, f.title AS ftitle, f.link AS flink FROM {aggregator_category_item} c LEFT JOIN {aggregator_item} i ON c.iid = i.iid LEFT JOIN {aggregator_feed} f ON i.fid = f.fid WHERE cid = %d ORDER BY timestamp DESC, iid DESC';
+    $sql = 'SELECT i.*, f.title AS ftitle, f.link AS flink FROM {aggregator_category_item} c LEFT JOIN {aggregator_item} i ON c.iid = i.iid LEFT JOIN {aggregator_feed} f ON i.fid = f.fid WHERE cid = %d ORDER BY timestamp DESC, i.iid DESC';
     $result = db_query_range($sql, $category->cid, 0, variable_get('feed_default_items', 10));
   }
   // or, get the default aggregator items

modules/locale/locale.module

@@ -366,7 +366,7 @@ function locale_refresh_cache() {
  * Returns plural form index for a specific number.
  *
  * The index is computed from the formula of this language.
- * 
+ *
  * @param $count
  *   Number to return plural for.
  * @param $langcode
@@ -378,7 +378,7 @@ function locale_get_plural($count, $langcode = NULL) {
   static $locale_formula, $plurals = array();
 
   $langcode = $langcode ? $langcode : $language->language;
-  
+
   if (!isset($plurals[$langcode][$count])) {
     if (!isset($locale_formula)) {
       $language_list = language_list();

modules/system/system.install

@@ -3306,6 +3306,39 @@ function system_update_6021() {
   return $ret;
 }
 
+/**
+ * Update files tables to associate files to a uid by default instead of a nid.
+ * Rename file_revisions to upload since it should only be used by the upload
+ * module used by upload to link files to nodes.
+ */
+function system_update_6022() {
+  $ret = array();
+
+  // Rename the nid field to vid, add status and timestamp fields, and indexes.
+  db_drop_index($ret, 'files', 'nid');
+  db_change_field($ret, 'files', 'nid', 'uid', array('type' => 'int', 'unsigned' => TRUE, 'not null' => TRUE, 'default' => 0));
+  db_add_field($ret, 'files', 'status', array('type' => 'int', 'not null' => TRUE, 'default' => 0));
+  db_add_field($ret, 'files', 'timestamp', array('type' => 'int', 'unsigned' => TRUE, 'not null' => TRUE, 'default' => 0));
+  db_add_index($ret, 'files', 'uid', array('uid'));
+  db_add_index($ret, 'files', 'status', array('status'));
+  db_add_index($ret, 'files', 'timestamp', array('timestamp'));
+
+  // Rename the file_revisions table to upload then add nid column. Since we're
+  // changing the table name we need to drop and re-add the vid index so both
+  // pgsql ends up with the corect index name.
+  db_drop_index($ret, 'file_revisions', 'vid');
+  db_rename_table($ret, 'file_revisions', 'upload');
+  db_add_field($ret, 'upload', 'nid', array('type' => 'int', 'unsigned' => TRUE, 'not null' => TRUE, 'default' => 0));
+  db_add_index($ret, 'upload', 'nid', array('nid'));
+  db_add_index($ret, 'upload', 'vid', array('vid'));
+
+  // The nid column was renamed to uid. Use the old nid to find the node's uid.
+  $ret[] = update_sql('UPDATE {files} f JOIN {node} n ON f.uid = n.nid SET f.uid = n.uid');
+  // Use the existing vid to find the nid.
+  $ret[] = update_sql('UPDATE {upload} u JOIN {node_revisions} r ON u.vid = r.vid SET u.nid = r.nid');
+
+  return $ret;
+}
 /**
  * @} End of "defgroup updates-5.x-to-6.x"
  * The next series of updates should start at 7000.

modules/system/system.module

@@ -13,6 +13,9 @@
 define('DRUPAL_MINIMUM_PGSQL',  '7.4');   // If using PostgreSQL
 define('DRUPAL_MINIMUM_APACHE', '1.3');   // If using Apache
 
+// Maximum age of temporary files in seconds.
+define('DRUPAL_MAXIMUM_TEMP_FILE_AGE', 1440);
+
 /**
  * Implementation of hook_help().
  */
@@ -83,7 +86,7 @@ function system_theme() {
  * Implementation of hook_perm().
  */
 function system_perm() {
-  return array('administer site configuration', 'access administration pages', 'select different theme');
+  return array('administer site configuration', 'access administration pages', 'select different theme', 'administer files');
 }
 
 /**
@@ -2171,28 +2174,29 @@ function system_theme_settings($key = '') {
   $form['var'] = array('#type' => 'hidden', '#value' => $var);
 
   // Check for a new uploaded logo, and use that instead.
-  if ($file = file_check_upload('logo_upload')) {
-    if ($info = image_get_info($file->filepath)) {
-      $parts = pathinfo($file->filename);
-      $filename = ($key) ? str_replace('/', '_', $key) .'_logo.'. $parts['extension'] : 'logo.'. $parts['extension'];
-
-      if ($file = file_save_upload('logo_upload', $filename, 1)) {
-        $_POST['default_logo'] = 0;
-        $_POST['logo_path'] = $file->filepath;
-        $_POST['toggle_logo'] = 1;
-      }
-    }
-    else {
-      form_set_error('file_upload', t('Only JPEG, PNG and GIF images are allowed to be used as logos.'));
+  if ($file = file_save_upload('logo_upload', array('file_validate_is_image' => array()))) {
+    $parts = pathinfo($file->filename);
+    $filename = ($key) ? str_replace('/', '_', $key) .'_logo.'. $parts['extension'] : 'logo.'. $parts['extension'];
+
+    // The image was saved using file_save_upload() and was added to the
+    // files table as a temorary file. We'll make a copy and let the garbage
+    // collector delete the original upload.
+    if (file_copy($file, $filename, FILE_EXISTS_REPLACE)) {
+      $_POST['default_logo'] = 0;
+      $_POST['logo_path'] = $file->filepath;
+      $_POST['toggle_logo'] = 1;
     }
   }
 
   // Check for a new uploaded favicon, and use that instead.
-  if ($file = file_check_upload('favicon_upload')) {
+  if ($file = file_save_upload('favicon_upload')) {
     $parts = pathinfo($file->filename);
     $filename = ($key) ? str_replace('/', '_', $key) .'_favicon.'. $parts['extension'] : 'favicon.'. $parts['extension'];
 
-    if ($file = file_save_upload('favicon_upload', $filename, 1)) {
+    // The image was saved using file_save_upload() and was added to the
+    // files table as a temorary file. We'll make a copy and let the garbage
+    // collector delete the original upload.
+    if (file_copy($file, $filename)) {
       $_POST['default_favicon'] = 0;
       $_POST['favicon_path'] = $file->filepath;
       $_POST['toggle_favicon'] = 1;
@@ -2636,13 +2640,27 @@ function theme_system_admin_by_module($menu_items) {
 /**
  * Implementation of hook_cron().
  *
- * Remove older rows from flood table
+ * Remove older rows from flood and batch table. Remove old temporary files.
  */
 function system_cron() {
-  // Cleanup the flood
+  // Cleanup the flood.
   db_query('DELETE FROM {flood} WHERE timestamp < %d', time() - 3600);
-  // Cleanup the batch table
+  // Cleanup the batch table.
   db_query('DELETE FROM {batch} WHERE timestamp < %d', time() - 864000);
+
+  // Remove temporary files that are older than DRUPAL_MAXIMUM_TEMP_FILE_AGE.
+  $result = db_query('SELECT * FROM {files} WHERE status = %s and timestamp < %d', FILE_STATUS_TEMPORARY, time() - DRUPAL_MAXIMUM_TEMP_FILE_AGE);
+  while ($file = db_fetch_object($result)) {
+    if (file_exists($file->filepath)) {
+      // If files that exist cannot be deleted, continue so the database remains
+      // consistant.
+      if (!file_delete($file->filepath)) {
+        watchdog('file system', t('Could not delete temporary file "%path" during garbage collection', array('%path' => $file->filepath)), 'error');
+        continue;
+      }
+    }
+    db_query('DELETE FROM {files} WHERE fid = %d', $file->fid);
+  }
 }
 
 /**

modules/system/system.schema

@@ -32,26 +32,21 @@ function system_schema() {
 
   $schema['files'] = array(
     'fields' => array(
-      'fid'      => array('type' => 'serial', 'unsigned' => TRUE, 'not null' => TRUE),
-      'nid'      => array('type' => 'int', 'unsigned' => TRUE, 'not null' => TRUE, 'default' => 0),
-      'filename' => array('type' => 'varchar', 'length' => 255, 'not null' => TRUE, 'default' => ''),
-      'filepath' => array('type' => 'varchar', 'length' => 255, 'not null' => TRUE, 'default' => ''),
-      'filemime' => array('type' => 'varchar', 'length' => 255, 'not null' => TRUE, 'default' => ''),
-      'filesize' => array('type' => 'int', 'unsigned' => TRUE, 'not null' => TRUE, 'default' => 0)
-    ),
-    'indexes' => array('nid' => array('nid')),
-    'primary key' => array('fid'),
-  );
-
-  $schema['file_revisions'] = array(
-    'fields' => array(
-      'fid'         => array('type' => 'int', 'unsigned' => TRUE, 'not null' => TRUE, 'default' => 0),
-      'vid'         => array('type' => 'int', 'unsigned' => TRUE, 'not null' => TRUE, 'default' => 0),
-      'description' => array('type' => 'varchar', 'length' => 255, 'not null' => TRUE, 'default' => ''),
-      'list'        => array('type' => 'int', 'unsigned' => TRUE, 'not null' => TRUE, 'default' => 0, 'size' => 'tiny')
+      'fid'       => array('type' => 'serial', 'unsigned' => TRUE, 'not null' => TRUE),
+      'uid'       => array('type' => 'int', 'unsigned' => TRUE, 'not null' => TRUE, 'default' => 0),
+      'filename'  => array('type' => 'varchar', 'length' => 255, 'not null' => TRUE, 'default' => ''),
+      'filepath'  => array('type' => 'varchar', 'length' => 255, 'not null' => TRUE, 'default' => ''),
+      'filemime'  => array('type' => 'varchar', 'length' => 255, 'not null' => TRUE, 'default' => ''),
+      'filesize'  => array('type' => 'int', 'unsigned' => TRUE, 'not null' => TRUE, 'default' => 0),
+      'status'    => array('type' => 'int', 'not null' => TRUE, 'default' => 0),
+      'timestamp' => array('type' => 'int', 'unsigned' => TRUE, 'not null' => TRUE, 'default' => 0),
     ),
-    'primary key' => array('fid', 'vid'),
-    'indexes' => array('vid' => array('vid')),
+    'indexes' => array(
+      'uid'       => array('uid'),
+      'status'    => array('status'),
+      'timestamp' => array('timestamp'),
+    ),
+    'primary key' => array('fid'),
   );
 
   $schema['flood'] = array(

modules/upload/upload.install

+<?php
+// $Id$
+
+/**
+ * Implementation of hook_install().
+ */
+function upload_install() {
+  // Create tables.
+  drupal_install_schema('upload');
+}
+
+/**
+ * Implementation of hook_uninstall().
+ */
+function upload_uninstall() {
+  // Remove tables.
+  drupal_uninstall_schema('upload');
+}

modules/upload/upload.schema

+<?php
+// $Id$
+
+function upload_schema() {
+  $schema['upload'] = array(
+    'fields' => array(
+      'fid'         => array('type' => 'int', 'unsigned' => TRUE, 'not null' => TRUE, 'default' => 0),
+      'nid'         => array('type' => 'int', 'unsigned' => TRUE, 'not null' => TRUE, 'default' => 0),
+      'vid'         => array('type' => 'int', 'unsigned' => TRUE, 'not null' => TRUE, 'default' => 0),
+      'description' => array('type' => 'varchar', 'length' => 255, 'not null' => TRUE, 'default' => ''),
+      'list'        => array('type' => 'int', 'unsigned' => TRUE, 'not null' => TRUE, 'default' => 0, 'size' => 'tiny')
+    ),
+    'primary key' => array('fid', 'vid'),
+    'indexes' => array('vid' => array('vid'), 'nid' => array('nid')),
+  );
+
+  return $schema;
+}
+

modules/user/user.module

@@ -320,32 +320,22 @@ function user_validate_mail($mail) {
 
 function user_validate_picture(&$form, &$form_state, $form_values) {
   // If required, validate the uploaded picture.
-  if (isset($form['picture']) && ($file = file_check_upload('picture_upload'))) {
-    // Check that uploaded file is an image, with a maximum file size
-    // and maximum height/width.
+  $validators = array(
+    'file_validate_is_image' => array(),
+    'file_validate_image_resolution' => array(variable_get('user_picture_dimensions', '85x85')),
+    'file_validate_size' => array(variable_get('user_picture_file_size', '30') * 1024),
+  );
+  if ($file = file_save_upload('picture_upload', $validators)) {
+    // The image was saved using file_save_upload() and was added to the
+    // files table as a temorary file. We'll make a copy and let the garbage
+    // collector delete the original upload.
     $info = image_get_info($file->filepath);
-    list($maxwidth, $maxheight) = explode('x', variable_get('user_picture_dimensions', '85x85'));
-
-    if (!$info || !$info['extension']) {
-      form_set_error('picture_upload', t('The uploaded file was not an image.'));
-    }
-    else if (image_get_toolkit()) {
-      image_scale($file->filepath, $file->filepath, $maxwidth, $maxheight);
-    }
-    else if (filesize($file->filepath) > (variable_get('user_picture_file_size', '30') * 1000)) {
-      form_set_error('picture_upload', t('The uploaded image is too large; the maximum file size is %size kB.', array('%size' => variable_get('user_picture_file_size', '30'))));
+    $destination = variable_get('user_picture_path', 'pictures') .'/picture-'. $form['#uid'] .'.'. $info['extension'];
+    if (file_copy($file, $destination, FILE_EXISTS_REPLACE)) {
+      $form_values['picture'] = $file->filepath;
     }
-    else if ($info['width'] > $maxwidth || $info['height'] > $maxheight) {
-      form_set_error('picture_upload', t('The uploaded image is too large; the maximum dimensions are %dimensions pixels.', array('%dimensions' => variable_get('user_picture_dimensions', '85x85'))));
-    }
-
-    if (!form_get_errors()) {
-      if ($file = file_save_upload('picture_upload', variable_get('user_picture_path', 'pictures') .'/picture-'. $form['#uid'] .'.'. $info['extension'], 1)) {
-        $form_values['picture'] = $file->filepath;
-      }
-      else {
-        form_set_error('picture_upload', t("Failed to upload the picture image; the %directory directory doesn't exist or is not writable.", array('%directory' => variable_get('user_picture_path', 'pictures'))));
-      }
+    else {
+      form_set_error('picture_upload', t("Failed to upload the picture image; the %directory directory doesn't exist or is not writable.", array('%directory' => variable_get('user_picture_path', 'pictures'))));
     }
   }
 }

themes/bluemarine/style-rtl.css

@@ -3,35 +3,35 @@ body {
   direction: rtl;
 }
 #logo img {
-  float: right; 
+  float: right;
 }
 #menu {
-  padding: 0.5em 0.5em 0 0.5em; 
-  text-align: left; 
+  padding: 0.5em 0.5em 0 0.5em;
+  text-align: left;
 }
 #navlist {
-  padding: 0 0 1.2em 0.8em; 
+  padding: 0 0 1.2em 0.8em;
 }
 #subnavlist {
-  padding: 0.5em 0 0.4em 1.2em; 
+  padding: 0.5em 0 0.4em 1.2em;
 }
 ul.links li {
-  border-right: 1px solid #9cf; 
+  border-right: 1px solid #9cf;
   border-left: inherit;
 }
 .block, .box {
   padding: 0 1.5em 0 0;
 }
 .node .taxonomy {
-  padding-right: 1.5em; 
+  padding-right: 1.5em;
 }
 .node .picture {
   float: left;
 }
 .comment .new {
   text-align: left;
-  float: left; 
+  float: left;
 }
 .comment .picture {
-  float: left; 
+  float: left;
 }

themes/bluemarine/style.css

@@ -138,7 +138,7 @@ ul.links li.first {
   font-weight: bold;
 }
 .site-name {
-  margin: 0.6em 0 0 ; 
+  margin: 0.6em 0 0 ;
   padding: 0;
   font-size: 2em;
 }