diff options
| -rw-r--r-- | includes/database/database.inc | 2 | ||||
| -rw-r--r-- | includes/file.inc | 83 | ||||
| -rw-r--r-- | modules/file/tests/file.test | 10 | ||||
| -rw-r--r-- | modules/image/image.test | 4 | ||||
| -rw-r--r-- | modules/simpletest/drupal_web_test_case.php | 2 | ||||
| -rw-r--r-- | modules/simpletest/tests/file.test | 4 | ||||
| -rw-r--r-- | modules/system/system.test | 5 |
7 files changed, 85 insertions, 25 deletions
diff --git a/includes/database/database.inc b/includes/database/database.inc index 32e3ab814..53d626df2 100644 --- a/includes/database/database.inc +++ b/includes/database/database.inc @@ -814,7 +814,7 @@ abstract class DatabaseConnection extends PDO { * * Force all alias names to be strictly alphanumeric-plus-underscore. In * contrast to DatabaseConnection::escapeField() / - * DatabaseConnection::escapeTable(), this doesn't allow the period (".") + * DatabaseConnection::escapeTable(), this doesn't allow the period (".") * * @return * The sanitized field name string. diff --git a/includes/file.inc b/includes/file.inc index 6550ae673..a6f7097ec 100644 --- a/includes/file.inc +++ b/includes/file.inc @@ -202,6 +202,7 @@ function file_stream_wrapper_valid_scheme($scheme) { } } + /** * Returns the part of an URI after the schema. * @@ -257,6 +258,10 @@ function file_stream_wrapper_uri_normalize($uri) { $uri = $scheme . '://' . $target; } } + else { + // The default scheme is file:// + $url = 'file://' . $uri; + } return $uri; } @@ -594,9 +599,9 @@ function file_usage_list(stdClass $file) { * @param $module * The name of the module using the file. * @param $type - * The type of the object that contains the referenced file. + * The type of the object that contains the referenced file. * @param $id - * The unique, numeric ID of the object containing the referenced file. + * The unique, numeric ID of the object containing the referenced file. * @param $count * (optional) The number of references to add to the object. Defaults to 1. * @@ -710,6 +715,12 @@ function file_usage_delete(stdClass $file, $module, $type = NULL, $id = NULL, $c * @see hook_file_copy() */ function file_copy(stdClass $source, $destination = NULL, $replace = FILE_EXISTS_RENAME) { + if (!file_valid_uri($destination)) { + watchdog('file', 'File %file (%realpath) could not be copied, because the destination %destination is invalid. This is often caused by improper use of file_copy() or a missing stream wrapper.', array('%file' => $source->uri, '%realpath' => drupal_realpath($source->uri), '%destination' => $destination)); + drupal_set_message(t('The specified file %file could not be copied, because the destination is invalid. More information is available in the system log.', array('%file' => $source->uri)), 'error'); + return FALSE; + } + if ($uri = file_unmanaged_copy($source->uri, $destination, $replace)) { $file = clone $source; $file->fid = NULL; @@ -741,6 +752,28 @@ function file_copy(stdClass $source, $destination = NULL, $replace = FILE_EXISTS } /** + * Determine whether the URI has a valid scheme for file API operations. + * + * There must be a scheme and it must be a Drupal-provided scheme like + * 'public', 'private', 'temporary', or an extension provided with + * hook_stream_wrappers(). + * + * @param $uri + * The URI to be tested. + * + * @return + * TRUE if the URI is allowed. + */ +function file_valid_uri($uri) { + // Assert that the URI has an allowed scheme. Barepaths are not allowed. + $uri_scheme = file_uri_scheme($uri); + if (empty($uri_scheme) || !file_stream_wrapper_valid_scheme($uri_scheme)) { + return FALSE; + } + return TRUE; +} + +/** * Copies a file to a new location without invoking the file API. * * This is a powerful function that in many ways performs like an advanced @@ -752,11 +785,12 @@ function file_copy(stdClass $source, $destination = NULL, $replace = FILE_EXISTS * replace the file or rename the file based on the $replace parameter. * * @param $source - * A string specifying the filepath or URI of the original file. + * A string specifying the filepath or URI of the source file. * @param $destination - * A URI containing the destination that $source should be copied to. - * This must be a stream wrapper URI. If this value is omitted, Drupal's - * default files scheme will be used, usually "public://". + * A URI containing the destination that $source should be copied to. The + * URI may be a bare filepath (without a scheme) and in that case the default + * scheme (file://) will be used. If this value is omitted, Drupal's default + * files scheme will be used, usually "public://". * @param $replace * Replace behavior when the destination file already exists: * - FILE_EXISTS_REPLACE - Replace the existing file. @@ -778,6 +812,7 @@ function file_unmanaged_copy($source, $destination = NULL, $replace = FILE_EXIST if (!file_exists($source)) { // @todo Replace drupal_set_message() calls with exceptions instead. drupal_set_message(t('The specified file %file could not be copied, because no file by that name exists. Please check that you supplied the correct filename.', array('%file' => $original_source)), 'error'); + watchdog('file', 'File %file (%realpath) could not be copied because it does not exist.', array('%file' => $original_source, '%realpath' => drupal_realpath($original_source))); return FALSE; } @@ -786,11 +821,6 @@ function file_unmanaged_copy($source, $destination = NULL, $replace = FILE_EXIST $destination = file_build_uri(basename($source)); } - // Assert that the destination contains a valid stream. - $destination_scheme = file_uri_scheme($destination); - if (!$destination_scheme || !file_stream_wrapper_valid_scheme($destination_scheme)) { - drupal_set_message(t('The specified file %file could not be copied, because the destination %destination is invalid. This is often caused by improper use of file_unmanaged_copy() or a missing stream wrapper.', array('%file' => $original_source, '%destination' => $destination)), 'error'); - } // Prepare the destination directory. if (file_prepare_directory($destination)) { @@ -802,7 +832,8 @@ function file_unmanaged_copy($source, $destination = NULL, $replace = FILE_EXIST $dirname = drupal_dirname($destination); if (!file_prepare_directory($dirname)) { // The destination is not valid. - drupal_set_message(t('The specified file %file could not be copied, because the destination %directory is not properly configured. This is often caused by a problem with file or directory permissions.', array('%file' => $original_source, '%directory' => $destination)), 'error'); + watchdog('file', 'File %file could not be copied, because the destination directory %directory is not configured correctly.', array('%file' => $original_source, '%destination' => drupal_realpath($dirname))); + drupal_set_message(t('The specified file %file could not be copied, because the destination directory is not properly configured. This may be caused by a problem with file or directory permissions. More information is available in the system log.', array('%file' => $original_source)), 'error'); return FALSE; } } @@ -810,20 +841,23 @@ function file_unmanaged_copy($source, $destination = NULL, $replace = FILE_EXIST // Determine whether we can perform this operation based on overwrite rules. $destination = file_destination($destination, $replace); if ($destination === FALSE) { - drupal_set_message(t('The file %file could not be copied because a file by that name already exists in the destination directory (%directory)', array('%file' => $source, '%directory' => $destination)), 'error'); + drupal_set_message(t('The file %file could not be copied because a file by that name already exists in the destination directory.', array('%file' => $original_source)), 'error'); + watchdog('file', 'File %file could not be copied because a file by that name already exists in the destination directory (%directory)', array('%file' => $original_source, '%destination' => drupal_realpath($destination))); return FALSE; } // Assert that the source and destination filenames are not the same. if (drupal_realpath($source) == drupal_realpath($destination)) { drupal_set_message(t('The specified file %file was not copied because it would overwrite itself.', array('%file' => $source)), 'error'); + watchdog('file', 'File %file could not be copied because it would overwrite itself.', array('%file' => $source)); return FALSE; } // Make sure the .htaccess files are present. file_ensure_htaccess(); // Perform the copy operation. if (!@copy($source, $destination)) { - watchdog('file', 'The specified file %file could not be copied to %destination.', array('%file' => $source, '%destination' => $destination), WATCHDOG_ERROR); + drupal_set_message(t('The specified file %file could not be copied.', array('%file' => $source)), 'error'); + watchdog('file', 'The specified file %file could not be copied to %destination.', array('%file' => $source, '%destination' => drupal_realpath($destination)), WATCHDOG_ERROR); return FALSE; } @@ -914,6 +948,12 @@ function file_destination($destination, $replace) { * @see hook_file_move() */ function file_move(stdClass $source, $destination = NULL, $replace = FILE_EXISTS_RENAME) { + if (!file_valid_uri($destination)) { + watchdog('file', 'File %file (%realpath) could not be moved, because the destination %destination is invalid. This may be caused by improper use of file_move() or a missing stream wrapper.', array('%file' => $source->uri, '%realpath' => drupal_realpath($source->uri), '%destination' => $destination)); + drupal_set_message(t('The specified file %file could not be moved, because the destination is invalid. More information is available in the system log.', array('%file' => $source->uri)), 'error'); + return FALSE; + } + if ($uri = file_unmanaged_move($source->uri, $destination, $replace)) { $delete_source = FALSE; @@ -1134,6 +1174,12 @@ function file_create_filename($basename, $directory) { * @see hook_file_delete() */ function file_delete(stdClass $file, $force = FALSE) { + if (!file_valid_uri($file->uri)) { + watchdog('file', 'File %file (%realpath) could not be deleted because it is not a valid URI. This may be caused by improper use of file_delete() or a missing stream wrapper.', array('%file' => $file->uri, '%realpath' => drupal_realpath($file->uri))); + drupal_set_message(t('The specified file %file could not be deleted, because it is not a valid URI. More information is available in the system log.', array('%file' => $file->uri)), 'error'); + return FALSE; + } + // If any module still has a usage entry in the file_usage table, the file // will not be deleted, but file_delete() will return a populated array // that tests as TRUE. @@ -1689,6 +1735,15 @@ function file_validate_image_resolution(stdClass $file, $maximum_dimensions = 0, function file_save_data($data, $destination = NULL, $replace = FILE_EXISTS_RENAME) { global $user; + if (empty($destination)) { + $destination = file_default_scheme() . '://'; + } + if (!file_valid_uri($destination)) { + watchdog('file', 'The data could not be saved because the destination %destination is invalid. This may be caused by improper use of file_save_data() or a missing stream wrapper.', array('%destination' => $destination)); + drupal_set_message(t('The data could not be saved, because the destination is invalid. More information is available in the system log.'), 'error'); + return FALSE; + } + if ($uri = file_unmanaged_save_data($data, $destination, $replace)) { // Create a file object. $file = new stdClass(); diff --git a/modules/file/tests/file.test b/modules/file/tests/file.test index 8330e8fa6..b650e605c 100644 --- a/modules/file/tests/file.test +++ b/modules/file/tests/file.test @@ -116,11 +116,11 @@ class FileFieldTestCase extends DrupalWebTestCase { if (is_numeric($nid_or_type)) { $node = node_load($nid_or_type); $delta = isset($node->$field_name) ? count($node->$field_name) : 0; - $edit['files[' . $field_name . '_' . LANGUAGE_NONE . '_' . $delta . ']'] = realpath($file->uri); + $edit['files[' . $field_name . '_' . LANGUAGE_NONE . '_' . $delta . ']'] = drupal_realpath($file->uri); $this->drupalPost('node/' . $nid_or_type . '/edit', $edit, t('Save')); } else { - $edit['files[' . $field_name . '_' . LANGUAGE_NONE . '_0]'] = realpath($file->uri); + $edit['files[' . $field_name . '_' . LANGUAGE_NONE . '_0]'] = drupal_realpath($file->uri); $type_name = str_replace('_', '-', $nid_or_type); $this->drupalPost('node/add/' . $type_name, $edit, t('Save')); } @@ -149,7 +149,7 @@ class FileFieldTestCase extends DrupalWebTestCase { */ function replaceNodeFile($file, $field_name, $nid, $new_revision = TRUE) { $edit = array( - 'files[' . $field_name . '_' . LANGUAGE_NONE . '_0]' => realpath($file->uri), + 'files[' . $field_name . '_' . LANGUAGE_NONE . '_0]' => drupal_realpath($file->uri), 'revision' => (string) (int) $new_revision, ); @@ -336,7 +336,7 @@ class FileFieldWidgetTestCase extends FileFieldTestCase { // Add a comment with a file. $text_file = $this->getTestFile('text'); $edit = array( - 'files[field_' . $name . '_' . LANGUAGE_NONE . '_' . 0 . ']' => realpath($text_file->uri), + 'files[field_' . $name . '_' . LANGUAGE_NONE . '_' . 0 . ']' => drupal_realpath($text_file->uri), 'comment_body[' . LANGUAGE_NONE . '][0][value]' => $comment_body = $this->randomName(), ); $this->drupalPost(NULL, $edit, t('Save')); @@ -566,6 +566,8 @@ class FileFieldValidateTestCase extends FileFieldTestCase { // Create a new node with the uploaded file. $nid = $this->uploadNodeFile($test_file, $field_name, $type_name); + $this->assertTrue($nid !== FALSE, t('uploadNodeFile(@test_file, @field_name, @type_name) succeeded', array('@test_file' => $test_file->uri, '@field_name' => $field_name, '@type_name' => $type_name))); + $node = node_load($nid, NULL, TRUE); $node_file = (object) $node->{$field_name}[LANGUAGE_NONE][0]; diff --git a/modules/image/image.test b/modules/image/image.test index 522524ab8..0709d918b 100644 --- a/modules/image/image.test +++ b/modules/image/image.test @@ -94,7 +94,7 @@ class ImageFieldTestCase extends DrupalWebTestCase { $edit = array( 'title' => $this->randomName(), ); - $edit['files[' . $field_name . '_' . LANGUAGE_NONE . '_0]'] = realpath($image->uri); + $edit['files[' . $field_name . '_' . LANGUAGE_NONE . '_0]'] = drupal_realpath($image->uri); $this->drupalPost('node/add/' . $type, $edit, t('Save')); // Retrieve ID of the newly created node from the current URL. @@ -786,7 +786,7 @@ class ImageFieldDisplayTestCase extends ImageFieldTestCase { // Add a default image to the imagefield instance. $images = $this->drupalGetTestFiles('image'); $edit = array( - 'files[field_settings_default_image]' => realpath($images[0]->uri), + 'files[field_settings_default_image]' => drupal_realpath($images[0]->uri), ); $this->drupalPost('admin/structure/types/manage/article/fields/' . $field_name, $edit, t('Save settings')); // Clear field info cache so the new default image is detected. diff --git a/modules/simpletest/drupal_web_test_case.php b/modules/simpletest/drupal_web_test_case.php index 323825ac2..36568a8d4 100644 --- a/modules/simpletest/drupal_web_test_case.php +++ b/modules/simpletest/drupal_web_test_case.php @@ -939,7 +939,7 @@ class DrupalWebTestCase extends DrupalTestCase { $files = array(); // Make sure type is valid. if (in_array($type, array('binary', 'html', 'image', 'javascript', 'php', 'sql', 'text'))) { - $files = file_scan_directory(variable_get('file_public_path', conf_path() . '/files'), '/' . $type . '\-.*/'); + $files = file_scan_directory('public://', '/' . $type . '\-.*/'); // If size is set then remove any files that are not of that size. if ($size !== NULL) { diff --git a/modules/simpletest/tests/file.test b/modules/simpletest/tests/file.test index 7d3c7bccc..f8c72ac63 100644 --- a/modules/simpletest/tests/file.test +++ b/modules/simpletest/tests/file.test @@ -575,7 +575,9 @@ class FileSaveUploadTest extends FileHookTestCase { $account = $this->drupalCreateUser(array('access content')); $this->drupalLogin($account); - $this->image = current($this->drupalGetTestFiles('image')); + $image_files = $this->drupalGetTestFiles('image'); + $this->image = current($image_files); + list(, $this->image_extension) = explode('.', $this->image->filename); $this->assertTrue(is_file($this->image->uri), t("The image file we're going to upload exists.")); diff --git a/modules/system/system.test b/modules/system/system.test index 9c2558de4..5e75910d1 100644 --- a/modules/system/system.test +++ b/modules/system/system.test @@ -1299,13 +1299,14 @@ class SystemThemeFunctionalTest extends DrupalWebTestCase { function testThemeSettings() { // Specify a filesystem path to be used for the logo. $file = current($this->drupalGetTestFiles('image')); + $fullpath = drupal_realpath($file->uri); $edit = array( 'default_logo' => FALSE, - 'logo_path' => $file->uri, + 'logo_path' => $fullpath, ); $this->drupalPost('admin/appearance/settings', $edit, t('Save configuration')); $this->drupalGet('node'); - $this->assertRaw($file->uri, t('Logo path successfully changed.')); + $this->assertRaw($fullpath, t('Logo path successfully changed.')); // Upload a file to use for the logo. $file = current($this->drupalGetTestFiles('image')); |