includes/file.inc | 43 ++++++--- includes/stream_wrappers.inc | 6 +- modules/simpletest/tests/file.test | 139 +++++++++++++++++++++++++++-- modules/simpletest/tests/file_test.module | 17 ++++ modules/system/image.gd.inc | 2 +- 5 files changed, 186 insertions(+), 21 deletions(-) diff --git a/includes/file.inc b/includes/file.inc index 7fd97ef..4b52e2f 100644 --- a/includes/file.inc +++ b/includes/file.inc @@ -755,7 +755,12 @@ function file_usage_delete(stdClass $file, $module, $type = NULL, $id = NULL, $c */ 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)); + if ($realpath = drupal_realpath($source->uri)) { + 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' => $realpath, '%destination' => $destination)); + } + else { + watchdog('file', 'File %file 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, '%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; } @@ -847,11 +852,15 @@ function file_unmanaged_copy($source, $destination = NULL, $replace = FILE_EXIST $original_destination = $destination; // Assert that the source file actually exists. - $source = drupal_realpath($source); 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))); + if ($realpath = drupal_realpath($original_source)) { + watchdog('file', 'File %file (%realpath) could not be copied because it does not exist.', array('%file' => $original_source, '%realpath' => $realpath)); + } + else { + watchdog('file', 'File %file could not be copied because it does not exist.', array('%file' => $original_source)); + } return FALSE; } @@ -871,7 +880,7 @@ function file_unmanaged_copy($source, $destination = NULL, $replace = FILE_EXIST $dirname = drupal_dirname($destination); if (!file_prepare_directory($dirname)) { // The destination is not valid. - watchdog('file', 'File %file could not be copied, because the destination directory %destination is not configured correctly.', array('%file' => $original_source, '%destination' => drupal_realpath($dirname))); + watchdog('file', 'File %file could not be copied, because the destination directory %destination is not configured correctly.', array('%file' => $original_source, '%destination' => $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; } @@ -881,12 +890,14 @@ function file_unmanaged_copy($source, $destination = NULL, $replace = FILE_EXIST $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.', 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))); + 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' => $destination)); return FALSE; } // Assert that the source and destination filenames are not the same. - if (drupal_realpath($source) == drupal_realpath($destination)) { + $real_source = drupal_realpath($source); + $real_destination = drupal_realpath($destination); + if ($source == $destination || ($real_source !== FALSE) && ($real_destination !== FALSE) && ($real_source == $real_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; @@ -895,7 +906,7 @@ function file_unmanaged_copy($source, $destination = NULL, $replace = FILE_EXIST 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' => drupal_realpath($destination)), LOG_ERR); + watchdog('file', 'The specified file %file could not be copied to %destination.', array('%file' => $source, '%destination' => $destination), LOG_ERR); return FALSE; } @@ -986,7 +997,12 @@ function file_destination($destination, $replace) { */ 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)); + if ($realpath = drupal_realpath($source->uri)) { + 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' => $realpath, '%destination' => $destination)); + } + else { + watchdog('file', 'File %file 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, '%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; } @@ -1212,7 +1228,12 @@ function file_create_filename($basename, $directory) { */ 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))); + if ($realpath = drupal_realpath($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' => $realpath)); + } + else { + watchdog('file', 'File %file 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)); + } 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; } @@ -1256,8 +1277,6 @@ function file_delete(stdClass $file, $force = FALSE) { * @see file_unmanaged_delete_recursive() */ function file_unmanaged_delete($path) { - // Resolve streamwrapper URI to local path. - $path = drupal_realpath($path); if (is_dir($path)) { watchdog('file', '%path is a directory and cannot be removed using file_unmanaged_delete().', array('%path' => $path), LOG_ERR); return FALSE; @@ -1299,8 +1318,6 @@ function file_unmanaged_delete($path) { * @see file_unmanaged_delete() */ function file_unmanaged_delete_recursive($path) { - // Resolve streamwrapper URI to local path. - $path = drupal_realpath($path); if (is_dir($path)) { $dir = dir($path); while (($entry = $dir->read()) !== FALSE) { diff --git a/includes/stream_wrappers.inc b/includes/stream_wrappers.inc index 7df1f9d..3c88f3d 100644 --- a/includes/stream_wrappers.inc +++ b/includes/stream_wrappers.inc @@ -341,7 +341,11 @@ abstract class DrupalLocalStreamWrapper implements DrupalStreamWrapperInterface * Base implementation of chmod(). */ function chmod($mode) { - return @chmod($this->getLocalPath(), $mode); + $output = @chmod($this->getLocalPath(), $mode); + // We are modifying the underlying file here, so we have to clear the stat + // cache so that PHP understands that URI has changed too. + clearstatcache(); + return $output; } /** diff --git a/modules/simpletest/tests/file.test b/modules/simpletest/tests/file.test index dc12b1b..9dbe546 100644 --- a/modules/simpletest/tests/file.test +++ b/modules/simpletest/tests/file.test @@ -196,10 +196,13 @@ class FileTestCase extends DrupalWebTestCase { * @return * File object. */ - function createFile($filepath = NULL, $contents = NULL, $scheme = 'public') { + function createFile($filepath = NULL, $contents = NULL, $scheme = NULL) { if (!isset($filepath)) { $filepath = $this->randomName(); } + if (!isset($scheme)) { + $scheme = file_default_scheme(); + } $filepath = $scheme . '://' . $filepath; if (!isset($contents)) { @@ -427,7 +430,7 @@ class FileValidatorTest extends DrupalWebTestCase { // Maximum size. if (image_get_toolkit()) { // Copy the image so that the original doesn't get resized. - copy(drupal_realpath('misc/druplicon.png'), 'temporary://druplicon.png'); + copy('misc/druplicon.png', 'temporary://druplicon.png'); $this->image->uri = 'temporary://druplicon.png'; $errors = file_validate_image_resolution($this->image, '10x5'); @@ -437,7 +440,7 @@ class FileValidatorTest extends DrupalWebTestCase { $this->assertTrue($info['width'] <= 10, t('Image scaled to correct width.'), 'File'); $this->assertTrue($info['height'] <= 5, t('Image scaled to correct height.'), 'File'); - drupal_unlink(drupal_realpath('temporary://druplicon.png')); + drupal_unlink('temporary://druplicon.png'); } else { // TODO: should check that the error is returned if no toolkit is available. @@ -531,18 +534,34 @@ class FileUnmanagedSaveDataTest extends FileTestCase { $filepath = file_unmanaged_save_data($contents); $this->assertTrue($filepath, t('Unnamed file saved correctly.')); $this->assertEqual(file_uri_scheme($filepath), file_default_scheme(), t("File was placed in Drupal's files directory.")); - $this->assertEqual($contents, file_get_contents(drupal_realpath($filepath)), t('Contents of the file are correct.')); + $this->assertEqual($contents, file_get_contents($filepath), t('Contents of the file are correct.')); // Provide a filename. $filepath = file_unmanaged_save_data($contents, 'public://asdf.txt', FILE_EXISTS_REPLACE); $this->assertTrue($filepath, t('Unnamed file saved correctly.')); $this->assertEqual('asdf.txt', basename($filepath), t('File was named correctly.')); - $this->assertEqual($contents, file_get_contents(drupal_realpath($filepath)), t('Contents of the file are correct.')); + $this->assertEqual($contents, file_get_contents($filepath), t('Contents of the file are correct.')); $this->assertFilePermissions($filepath, variable_get('file_chmod_file', 0664)); } } /** + * Tests the file_unmanaged_save_data() function on remote filesystems. + */ +class RemoteFileUnmanagedSaveDataTest extends FileUnmanagedSaveDataTest { + public static function getInfo() { + $info = parent::getInfo(); + $info['group'] = 'File API (remote)'; + return $info; + } + + function setUp() { + parent::setUp('file_test'); + variable_set('file_default_scheme', 'dummy-remote'); + } +} + +/** * Test the file_save_upload() function. */ class FileSaveUploadTest extends FileHookTestCase { @@ -863,6 +882,22 @@ class FileSaveUploadTest extends FileHookTestCase { } /** + * Test the file_save_upload() function on remote filesystems. + */ +class RemoteFileSaveUploadTest extends FileSaveUploadTest { + public static function getInfo() { + $info = parent::getInfo(); + $info['group'] = 'File API (remote)'; + return $info; + } + + function setUp() { + parent::setUp('file_test'); + variable_set('file_default_scheme', 'dummy-remote'); + } +} + +/** * Directory related tests. */ class FileDirectoryTest extends FileTestCase { @@ -879,7 +914,7 @@ class FileDirectoryTest extends FileTestCase { */ function testFileCheckDirectoryHandling() { // A directory to operate on. - $directory = file_stream_wrapper_get_instance_by_scheme(file_default_scheme())->getDirectoryPath() . '/' . $this->randomName() . '/' . $this->randomName(); + $directory = file_default_scheme() . '://' . $this->randomName() . '/' . $this->randomName(); $this->assertFalse(is_dir($directory), t('Directory does not exist prior to testing.')); // Non-existent directory. @@ -986,6 +1021,22 @@ class FileDirectoryTest extends FileTestCase { } /** + * Directory related tests. + */ +class RemoteFileDirectoryTest extends FileDirectoryTest { + public static function getInfo() { + $info = parent::getInfo(); + $info['group'] = 'File API (remote)'; + return $info; + } + + function setUp() { + parent::setUp('file_test'); + variable_set('file_default_scheme', 'dummy-remote'); + } +} + +/** * Tests the file_scan_directory() function. */ class FileScanDirectoryTest extends FileTestCase { @@ -1114,6 +1165,21 @@ class FileScanDirectoryTest extends FileTestCase { } } +/** + * Tests the file_scan_directory() function on remote filesystems. + */ +class RemoteFileScanDirectoryTest extends FileScanDirectoryTest { + public static function getInfo() { + $info = parent::getInfo(); + $info['group'] = 'File API (remote)'; + return $info; + } + + function setUp() { + parent::setUp('file_test'); + variable_set('file_default_scheme', 'dummy-remote'); + } +} /** * Deletion related tests. @@ -1160,6 +1226,21 @@ class FileUnmanagedDeleteTest extends FileTestCase { } } +/** + * Deletion related tests on remote filesystems. + */ +class RemoteFileUnmanagedDeleteTest extends FileUnmanagedDeleteTest { + public static function getInfo() { + $info = parent::getInfo(); + $info['group'] = 'File API (remote)'; + return $info; + } + + function setUp() { + parent::setUp('file_test'); + variable_set('file_default_scheme', 'dummy-remote'); + } +} /** * Deletion related tests. @@ -1237,6 +1318,21 @@ class FileUnmanagedDeleteRecursiveTest extends FileTestCase { } } +/** + * Deletion related tests on remote filesystems. + */ +class RemoteFileUnmanagedDeleteRecursiveTest extends FileUnmanagedDeleteRecursiveTest { + public static function getInfo() { + $info = parent::getInfo(); + $info['group'] = 'File API (remote)'; + return $info; + } + + function setUp() { + parent::setUp('file_test'); + variable_set('file_default_scheme', 'dummy-remote'); + } +} /** * Unmanaged move related tests. @@ -1310,6 +1406,21 @@ class FileUnmanagedMoveTest extends FileTestCase { } } +/** + * Unmanaged move related tests on remote filesystems. + */ +class RemoteFileUnmanagedMoveTest extends FileUnmanagedMoveTest { + public static function getInfo() { + $info = parent::getInfo(); + $info['group'] = 'File API (remote)'; + return $info; + } + + function setUp() { + parent::setUp('file_test'); + variable_set('file_default_scheme', 'dummy-remote'); + } +} /** * Unmanaged copy related tests. @@ -1400,6 +1511,22 @@ class FileUnmanagedCopyTest extends FileTestCase { } /** + * Unmanaged copy related tests on remote filesystems. + */ +class RemoteFileUnmanagedCopyTest extends FileUnmanagedCopyTest { + public static function getInfo() { + $info = parent::getInfo(); + $info['group'] = 'File API (remote)'; + return $info; + } + + function setUp() { + parent::setUp('file_test'); + variable_set('file_default_scheme', 'dummy-remote'); + } +} + +/** * Deletion related tests. */ class FileDeleteTest extends FileHookTestCase { diff --git a/modules/simpletest/tests/file_test.module b/modules/simpletest/tests/file_test.module index 2865a1f..b3c43e0 100644 --- a/modules/simpletest/tests/file_test.module +++ b/modules/simpletest/tests/file_test.module @@ -37,6 +37,11 @@ function file_test_stream_wrappers() { 'class' => 'DrupalDummyStreamWrapper', 'description' => t('Dummy wrapper for simpletest.'), ), + 'dummy-remote' => array( + 'name' => t('Dummy files (remote)'), + 'class' => 'DrupalDummyRemoteStreamWrapper', + 'description' => t('Dummy wrapper for simpletest (remote).'), + ), ); } @@ -442,3 +447,15 @@ class DrupalDummyStreamWrapper extends DrupalLocalStreamWrapper { } } +/** + * Helper class for testing the stream wrapper registry. + * + * Dummy remote stream wrapper implementation (dummy-remote://). + * + * Basically just the public scheme but not returning a local file for realpath. + */ +class DrupalDummyRemoteStreamWrapper extends DrupalPublicStreamWrapper { + function realpath() { + return FALSE; + } +} diff --git a/modules/system/image.gd.inc b/modules/system/image.gd.inc index a3f76d4..39f86dc 100644 --- a/modules/system/image.gd.inc +++ b/modules/system/image.gd.inc @@ -346,7 +346,7 @@ function image_gd_create_tmp(stdClass $image, $width, $height) { */ function image_gd_get_info(stdClass $image) { $details = FALSE; - $data = getimagesize(drupal_realpath($image->source)); + $data = getimagesize($image->source); if (isset($data) && is_array($data)) { $extensions = array('1' => 'gif', '2' => 'jpg', '3' => 'png');