diff --git a/core/includes/file.inc b/core/includes/file.inc index db702d173a9..e1219b3fdf2 100644 --- a/core/includes/file.inc +++ b/core/includes/file.inc @@ -584,19 +584,23 @@ function file_ensure_htaccess() { * @param $private * FALSE indicates that $directory should be an open and public directory. * The default is TRUE which indicates a private and protected directory. + * + * @return bool + * TRUE if the .htaccess file could be created or existed already, FALSE + * otherwise. */ function file_save_htaccess($directory, $private = TRUE) { if (file_uri_scheme($directory)) { - $directory = file_stream_wrapper_uri_normalize($directory); + $htaccess_path = file_stream_wrapper_uri_normalize($directory . '/.htaccess'); } else { $directory = rtrim($directory, '/\\'); + $htaccess_path = $directory . '/.htaccess'; } - $htaccess_path = $directory . '/.htaccess'; if (file_exists($htaccess_path)) { // Short circuit if the .htaccess file already exists. - return; + return TRUE; } if ($private) { @@ -609,12 +613,13 @@ function file_save_htaccess($directory, $private = TRUE) { } // Write the .htaccess file. - if (file_put_contents($htaccess_path, $htaccess_lines)) { - drupal_chmod($htaccess_path, 0444); + if (file_exists($directory) && is_writable($directory) && file_put_contents($htaccess_path, $htaccess_lines)) { + return drupal_chmod($htaccess_path, 0444); } else { $variables = array('%directory' => $directory, '!htaccess' => '
' . nl2br(String::checkPlain($htaccess_lines))); watchdog('security', "Security warning: Couldn't write .htaccess file. Please create a .htaccess file in your %directory directory which contains the following lines: !htaccess", $variables, WATCHDOG_ERROR); + return FALSE; } } diff --git a/core/modules/system/lib/Drupal/system/Tests/File/HtaccessUnitTest.php b/core/modules/system/lib/Drupal/system/Tests/File/HtaccessUnitTest.php new file mode 100644 index 00000000000..5f4b6564c24 --- /dev/null +++ b/core/modules/system/lib/Drupal/system/Tests/File/HtaccessUnitTest.php @@ -0,0 +1,90 @@ + '.htaccess file saving', + 'description' => 'Tests .htaccess file saving.', + 'group' => 'File API', + ); + } + + /** + * Tests file_save_htaccess(). + */ + function testHtaccessSave() { + // Prepare test directories. + $public = $this->public_files_directory . '/test/public'; + $private = $this->public_files_directory . '/test/private'; + $stream = 'public://test/stream'; + + // Verify that file_save_htaccess() returns FALSE if .htaccess cannot be + // written. + // Note: We cannot test the condition of a directory lacking write + // permissions, since at least on Windows file_save_htaccess() succeeds + // even when changing directory permissions to 0000. + $this->assertFalse(file_save_htaccess($public, FALSE)); + + // Create public .htaccess file. + mkdir($public, 0777, TRUE); + $this->assertTrue(file_save_htaccess($public, FALSE)); + $content = file_get_contents($public . '/.htaccess'); + $this->assertIdentical($content, "SetHandler Drupal_Security_Do_Not_Remove_See_SA_2006_006\nOptions None\nOptions +FollowSymLinks"); + $this->assertFilePermissions($public . '/.htaccess', 0444); + + $this->assertTrue(file_save_htaccess($public, FALSE)); + + // Create private .htaccess file. + mkdir($private, 0777, TRUE); + $this->assertTrue(file_save_htaccess($private)); + $content = file_get_contents($private . '/.htaccess'); + $this->assertIdentical($content, "SetHandler Drupal_Security_Do_Not_Remove_See_SA_2006_006\nDeny from all\nOptions None\nOptions +FollowSymLinks"); + $this->assertFilePermissions($private . '/.htaccess', 0444); + + $this->assertTrue(file_save_htaccess($private)); + + // Create an .htaccess file using a stream URI. + mkdir($stream, 0777, TRUE); + $this->assertTrue(file_save_htaccess($stream)); + $content = file_get_contents($stream . '/.htaccess'); + $this->assertIdentical($content, "SetHandler Drupal_Security_Do_Not_Remove_See_SA_2006_006\nDeny from all\nOptions None\nOptions +FollowSymLinks"); + $this->assertFilePermissions($stream . '/.htaccess', 0444); + + $this->assertTrue(file_save_htaccess($stream)); + } + + /** + * Asserts expected file permissions for a given file. + * + * @param string $uri + * The URI of the file to check. + * @param int $expected + * The expected file permissions; e.g., 0444. + * + * @return bool + * Whether the actual file permissions match the expected. + */ + protected function assertFilePermissions($uri, $expected) { + $actual = fileperms($uri) & 0777; + return $this->assertIdentical($actual, $expected, String::format('@uri file permissions @actual are identical to @expected.', array( + '@uri' => $uri, + '@actual' => 0 . decoct($actual), + '@expected' => 0 . decoct($expected), + ))); + } + +}