From 755913e0b7fc552126b3cf2d5074b911f24f996b Mon Sep 17 00:00:00 2001 From: Dries Buytaert <dries@buytaert.net> Date: Tue, 17 Aug 2010 22:05:22 +0000 Subject: [PATCH] - Patch #443286 by c960657, Damien Tournoud, drifter, webkenny, scor: international characters break file handling. --- includes/file.inc | 70 +++++++++++++++++++++++++-- includes/filetransfer/ftp.inc | 4 +- includes/filetransfer/local.inc | 8 +-- includes/stream_wrappers.inc | 6 +-- modules/color/color.module | 4 +- modules/locale/locale.install | 2 +- modules/locale/locale.test | 4 +- modules/simpletest/simpletest.install | 2 +- modules/simpletest/tests/file.test | 53 ++++++++++++++++---- modules/system/system.install | 4 +- modules/system/system.tar.inc | 12 ++--- 11 files changed, 133 insertions(+), 36 deletions(-) diff --git a/includes/file.inc b/includes/file.inc index 0d4af76931b7..328cf08de414 100644 --- a/includes/file.inc +++ b/includes/file.inc @@ -943,6 +943,10 @@ function file_create_filename($basename, $directory) { // Strip control characters (ASCII value < 32). Though these are allowed in // some filesystems, not many applications handle them well. $basename = preg_replace('/[\x00-\x1F]/u', '_', $basename); + if (substr(PHP_OS, 0, 3) == 'WIN') { + // These characters are not allowed in Windows filenames + $basename = str_replace(array(':', '*', '?', '"', '<', '>', '|'), '_', $basename); + } // A URI or path may already have a trailing slash or look like "public://". if (substr($directory, -1) == '/') { @@ -1042,7 +1046,7 @@ function file_unmanaged_delete($path) { return FALSE; } if (is_file($path)) { - return unlink($path); + return drupal_unlink($path); } // Return TRUE for non-existent file, but log that nothing was actually // deleted, as the current state is the intended result. @@ -1090,7 +1094,8 @@ function file_unmanaged_delete_recursive($path) { file_unmanaged_delete_recursive($entry_path); } $dir->close(); - return rmdir($path); + + return drupal_rmdir($path); } return file_unmanaged_delete($path); } @@ -1892,6 +1897,36 @@ function drupal_chmod($uri, $mode = NULL) { return FALSE; } +/** + * Deletes a file. + * + * PHP's unlink() is broken on Windows, as it can fail to remove a file + * when it has a read-only flag set. + * + * @param $uri + * A URI or pathname. + * @param $context + * Refer to http://php.net/manual/en/ref.stream.php + * + * @return + * Boolean TRUE on success, or FALSE on failure. + * + * @see unlink() + * @ingroup php_wrappers + */ +function drupal_unlink($uri, $context = NULL) { + $scheme = file_uri_scheme($uri); + if ((!$scheme || !file_stream_wrapper_valid_scheme($scheme)) && (substr(PHP_OS, 0, 3) == 'WIN')) { + chmod($uri, 0600); + } + if ($context) { + return unlink($uri, $context); + } + else { + return unlink($uri); + } +} + /** * Returns the absolute path of a file or directory * @@ -1988,7 +2023,6 @@ function drupal_dirname($uri) { * @ingroup php_wrappers */ function drupal_mkdir($uri, $mode = NULL, $recursive = FALSE, $context = NULL) { - if (is_null($mode)) { $mode = variable_get('file_chmod_directory', 0775); } @@ -2001,6 +2035,36 @@ function drupal_mkdir($uri, $mode = NULL, $recursive = FALSE, $context = NULL) { } } +/** + * Remove a directory. + * + * PHP's rmdir() is broken on Windows, as it can fail to remove a directory + * when it has a read-only flag set. + * + * @param $uri + * A URI or pathname. + * @param $context + * Refer to http://php.net/manual/en/ref.stream.php + * + * @return + * Boolean TRUE on success, or FALSE on failure. + * + * @see rmdir() + * @ingroup php_wrappers + */ +function drupal_rmdir($uri, $context = NULL) { + $scheme = file_uri_scheme($uri); + if ((!$scheme || !file_stream_wrapper_valid_scheme($scheme)) && (substr(PHP_OS, 0, 3) == 'WIN')) { + chmod($uri, 0700); + } + if ($context) { + return rmdir($uri, $context); + } + else { + return rmdir($uri); + } +} + /** * Creates a file with a unique filename in the specified directory. * diff --git a/includes/filetransfer/ftp.inc b/includes/filetransfer/ftp.inc index b6046b2def40..d370e09db867 100644 --- a/includes/filetransfer/ftp.inc +++ b/includes/filetransfer/ftp.inc @@ -77,7 +77,7 @@ function removeDirectoryJailed($directory) { } } closedir($dh); - if (!rmdir($this->connection . $directory)) { + if (!drupal_rmdir($this->connection . $directory)) { $exception = new FileTransferException('Cannot remove @directory.', NULL, array('@directory' => $directory)); throw $exception; } @@ -91,7 +91,7 @@ function copyFileJailed($source, $destination) { } function removeFileJailed($destination) { - if (!@unlink($this->connection . '/' .$destination)) { + if (!@drupal_unlink($this->connection . '/' .$destination)) { throw new FileTransferException('Cannot remove @destination', NULL, array('@destination' => $destination)); } } diff --git a/includes/filetransfer/local.inc b/includes/filetransfer/local.inc index 14e246671f65..8cad200b1ca7 100644 --- a/includes/filetransfer/local.inc +++ b/includes/filetransfer/local.inc @@ -33,23 +33,23 @@ protected function removeDirectoryJailed($directory) { } foreach (new RecursiveIteratorIterator(new RecursiveDirectoryIterator($directory), RecursiveIteratorIterator::CHILD_FIRST) as $filename => $file) { if ($file->isDir()) { - if (@!rmdir($filename)) { + if (@!drupal_rmdir($filename)) { throw new FileTransferException('Cannot remove directory %directory.', NULL, array('%directory' => $filename)); } } elseif ($file->isFile()) { - if (@!unlink($filename)) { + if (@!drupal_unlink($filename)) { throw new FileTransferException('Cannot remove file %file.', NULL, array('%file' => $filename)); } } } - if (@!rmdir($directory)) { + if (@!drupal_rmdir($directory)) { throw new FileTransferException('Cannot remove directory %directory.', NULL, array('%directory' => $directory)); } } protected function removeFileJailed($file) { - if (@!unlink($file)) { + if (@!drupal_unlink($file)) { throw new FileTransferException('Cannot remove file %file.', NULL, array('%file' => $file)); } } diff --git a/includes/stream_wrappers.inc b/includes/stream_wrappers.inc index a78374c967b1..f49d3bc294f3 100644 --- a/includes/stream_wrappers.inc +++ b/includes/stream_wrappers.inc @@ -539,7 +539,7 @@ public function stream_close() { */ public function unlink($uri) { $this->uri = $uri; - return unlink($this->getLocalPath()); + return drupal_unlink($this->getLocalPath()); } /** @@ -636,10 +636,10 @@ public function mkdir($uri, $mode, $options) { public function rmdir($uri, $options) { $this->uri = $uri; if ($options & STREAM_REPORT_ERRORS) { - return rmdir($this->getLocalPath()); + return drupal_rmdir($this->getLocalPath()); } else { - return @rmdir($this->getLocalPath()); + return @drupal_rmdir($this->getLocalPath()); } } diff --git a/modules/color/color.module b/modules/color/color.module index 6a3ca99c4c6f..01341a61811e 100644 --- a/modules/color/color.module +++ b/modules/color/color.module @@ -312,10 +312,10 @@ function color_scheme_form_submit($form, &$form_state) { // Delete old files. foreach (variable_get('color_' . $theme . '_files', array()) as $file) { - @unlink($file); + @drupal_unlink($file); } if (isset($file) && $file = dirname($file)) { - @rmdir($file); + @drupal_rmdir($file); } // Don't render the default colorscheme, use the standard theme instead. diff --git a/modules/locale/locale.install b/modules/locale/locale.install index f9c1c25d4635..221241605a53 100644 --- a/modules/locale/locale.install +++ b/modules/locale/locale.install @@ -129,7 +129,7 @@ function locale_uninstall() { } // Delete the JavaScript translations directory if empty. if (!file_scan_directory($locale_js_directory, '/.*/')) { - rmdir($locale_js_directory); + drupal_rmdir($locale_js_directory); } } diff --git a/modules/locale/locale.test b/modules/locale/locale.test index e6c7cac819e9..b2f36c6e2380 100644 --- a/modules/locale/locale.test +++ b/modules/locale/locale.test @@ -762,7 +762,7 @@ class LocaleImportFunctionalTest extends DrupalWebTestCase { file_put_contents($name, $contents); $options['files[file]'] = $name; $this->drupalPost('admin/config/regional/translate/import', $options, t('Import')); - unlink($name); + drupal_unlink($name); } /** @@ -911,7 +911,7 @@ class LocaleExportFunctionalTest extends DrupalWebTestCase { 'langcode' => 'fr', 'files[file]' => $name, ), t('Import')); - unlink($name); + drupal_unlink($name); // Get the French translations. $this->drupalPost('admin/config/regional/translate/export', array( diff --git a/modules/simpletest/simpletest.install b/modules/simpletest/simpletest.install index c7eea9f986eb..a3f2c72b6e8f 100644 --- a/modules/simpletest/simpletest.install +++ b/modules/simpletest/simpletest.install @@ -25,7 +25,7 @@ function simpletest_uninstall() { foreach ($files as $file) { file_unmanaged_delete($file->uri); } - rmdir($path); + drupal_rmdir($path); } /** diff --git a/modules/simpletest/tests/file.test b/modules/simpletest/tests/file.test index 4cfdad807cdf..cf661c63b7a7 100644 --- a/modules/simpletest/tests/file.test +++ b/modules/simpletest/tests/file.test @@ -108,7 +108,20 @@ class FileTestCase extends DrupalWebTestCase { clearstatcache(); // Mask out all but the last three octets. - $actual_mode = fileperms($filepath) & 511; + $actual_mode = fileperms($filepath) & 0777; + + // PHP on Windows has limited support for file permissions. Usually each of + // "user", "group" and "other" use one octal digit (3 bits) to represent the + // read/write/execute bits. On Windows, chmod() ignores the "group" and + // "other" bits, and fileperms() returns the "user" bits in all three + // positions. $expected_mode is updated to reflect this. + if (substr(PHP_OS, 0, 3) == 'WIN') { + // Reset the "group" and "other" bits. + $expected_mode = $expected_mode & 0700; + // Shift the "user" bits to the "group" and "other" positions also. + $expected_mode = $expected_mode | $expected_mode >> 3 | $expected_mode >> 6; + } + if (!isset($message)) { $message = t('Expected file permission to be %expected, actually were %actual.', array('%actual' => decoct($actual_mode), '%expected' => decoct($expected_mode))); } @@ -130,7 +143,20 @@ class FileTestCase extends DrupalWebTestCase { clearstatcache(); // Mask out all but the last three octets. - $actual_mode = fileperms($directory) & 511; + $actual_mode = fileperms($directory) & 0777; + + // PHP on Windows has limited support for file permissions. Usually each of + // "user", "group" and "other" use one octal digit (3 bits) to represent the + // read/write/execute bits. On Windows, chmod() ignores the "group" and + // "other" bits, and fileperms() returns the "user" bits in all three + // positions. $expected_mode is updated to reflect this. + if (substr(PHP_OS, 0, 3) == 'WIN') { + // Reset the "group" and "other" bits. + $expected_mode = $expected_mode & 0700; + // Shift the "user" bits to the "group" and "other" positions also. + $expected_mode = $expected_mode | $expected_mode >> 3 | $expected_mode >> 6; + } + if (!isset($message)) { $message = t('Expected directory permission to be %expected, actually were %actual.', array('%actual' => decoct($actual_mode), '%expected' => decoct($expected_mode))); } @@ -413,7 +439,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'); - unlink(drupal_realpath($temp_dir . '/druplicon.png')); + drupal_unlink(drupal_realpath($temp_dir . '/druplicon.png')); } else { // TODO: should check that the error is returned if no toolkit is available. @@ -866,19 +892,26 @@ class FileDirectoryTest extends FileTestCase { // Make sure directory actually exists. $this->assertTrue(is_dir($directory), t('Directory actually exists.'), 'File'); - // Make directory read only. - @chmod($directory, 0444); - $this->assertFalse(file_prepare_directory($directory, 0), t('Error reported for a non-writeable directory.'), 'File'); + if (substr(PHP_OS, 0, 3) != 'WIN') { + // PHP on Windows doesn't support any kind of useful read-only mode for + // directories. When executing a chmod() on a directory, PHP only sets the + // read-only flag, which doesn't prevent files to actually be written + // in the directory on any recent version of Windows. + + // Make directory read only. + @drupal_chmod($directory, 0444); + $this->assertFalse(file_prepare_directory($directory, 0), t('Error reported for a non-writeable directory.'), 'File'); - // Test directory permission modification. - $this->assertTrue(file_prepare_directory($directory, FILE_MODIFY_PERMISSIONS), t('No error reported when making directory writeable.'), 'File'); + // Test directory permission modification. + $this->assertTrue(file_prepare_directory($directory, FILE_MODIFY_PERMISSIONS), t('No error reported when making directory writeable.'), 'File'); + } - // Test directory permission modification actually set correct permissions. + // Test that the directory has the correct permissions. $this->assertDirectoryPermissions($directory, variable_get('file_chmod_directory', 0775)); // Remove .htaccess file to then test that it gets re-created. $directory = file_directory_path(); - @unlink($directory . '/.htaccess'); + @drupal_unlink($directory . '/.htaccess'); $this->assertFalse(is_file($directory . '/.htaccess'), t('Successfully removed the .htaccess file in the files directory.'), 'File'); file_ensure_htaccess(); $this->assertTrue(is_file($directory . '/.htaccess'), t('Successfully re-created the .htaccess file in the files directory.'), 'File'); diff --git a/modules/system/system.install b/modules/system/system.install index 492df8365fe3..d4da8b4a7929 100644 --- a/modules/system/system.install +++ b/modules/system/system.install @@ -2351,10 +2351,10 @@ function system_update_7046() { variable_set('theme_garland_settings', $settings); // Remove Garland's color files since they won't match Minnelli's. foreach (variable_get('color_garland_files', array()) as $file) { - @unlink($file); + @drupal_unlink($file); } if (isset($file) && $file = dirname($file)) { - @rmdir($file); + @drupal_rmdir($file); } variable_del('color_garland_palette'); variable_del('color_garland_stylesheets'); diff --git a/modules/system/system.tar.inc b/modules/system/system.tar.inc index 30b0c2f551ca..fd1d3c8256a6 100644 --- a/modules/system/system.tar.inc +++ b/modules/system/system.tar.inc @@ -210,7 +210,7 @@ function __destruct() $this->_close(); // ----- Look for a local copy to delete if ($this->_temp_tarname != '') - @unlink($this->_temp_tarname); + @drupal_unlink($this->_temp_tarname); // $this->_PEAR(); } // }}} @@ -777,7 +777,7 @@ function _close() // ----- Look if a local copy need to be erase // Note that it might be interesting to keep the url for a time : ToDo if ($this->_temp_tarname != '') { - @unlink($this->_temp_tarname); + @drupal_unlink($this->_temp_tarname); $this->_temp_tarname = ''; } @@ -793,11 +793,11 @@ function _cleanFile() // ----- Look for a local copy if ($this->_temp_tarname != '') { // ----- Remove the local copy but not the remote tarname - @unlink($this->_temp_tarname); + @drupal_unlink($this->_temp_tarname); $this->_temp_tarname = ''; } else { // ----- Remove the local tarname file - @unlink($this->_tarname); + @drupal_unlink($this->_tarname); } $this->_tarname = ''; @@ -1603,7 +1603,7 @@ function _extractList($p_path, &$p_list_detail, $p_mode, } } elseif ($v_header['typeflag'] == "2") { if (@file_exists($v_header['filename'])) { - @unlink($v_header['filename']); + @drupal_unlink($v_header['filename']); } if (!@symlink($v_header['link'], $v_header['filename'])) { $this->_error('Unable to extract symbolic link {' @@ -1740,7 +1740,7 @@ function _openAppend() @bzclose($v_temp_tar); } - if (!@unlink($this->_tarname.".tmp")) { + if (!@drupal_unlink($this->_tarname.".tmp")) { $this->_error('Error while deleting temporary file \'' .$this->_tarname.'.tmp\''); } -- GitLab