diff --git a/core/core.services.yml b/core/core.services.yml index e3bf98833ba..66a46c66036 100644 --- a/core/core.services.yml +++ b/core/core.services.yml @@ -424,9 +424,6 @@ services: logger.channel.form: parent: logger.channel_base arguments: ['form'] - logger.channel.security: - parent: logger.channel_base - arguments: ['security'] logger.log_message_parser: class: Drupal\Core\Logger\LogMessageParser @@ -1653,9 +1650,6 @@ services: - { name: twig.loader, priority: -100 } element_info: alias: plugin.manager.element_info - file.htaccess_writer: - class: Drupal\Core\File\HtaccessWriter - arguments: ['@file_system', '@logger.channel.security'] file.mime_type.guesser: class: Drupal\Core\File\MimeType\MimeTypeGuesser arguments: ['@stream_wrapper_manager'] diff --git a/core/includes/file.inc b/core/includes/file.inc index 9c67bb15189..77a743eab04 100644 --- a/core/includes/file.inc +++ b/core/includes/file.inc @@ -6,12 +6,13 @@ */ use Drupal\Component\FileSystem\FileSystem as ComponentFileSystem; +use Drupal\Component\Utility\UrlHelper; use Drupal\Component\PhpStorage\FileStorage; use Drupal\Component\Utility\Bytes; -use Drupal\Component\Utility\UrlHelper; use Drupal\Core\File\FileSystem; use Drupal\Core\Site\Settings; use Drupal\Core\StreamWrapper\PublicStream; +use Drupal\Core\StreamWrapper\PrivateStream; /** * Default mode for new directories. See drupal_chmod(). @@ -314,15 +315,30 @@ function file_prepare_directory(&$directory, $options = FILE_MODIFY_PERMISSIONS) /** * Creates a .htaccess file in each Drupal files directory if it is missing. - * - * @deprecated in Drupal 8.7.x, will be removed before Drupal 9.0.0. Use - * \Drupal\Core\File\HtaccessWriterInterface::ensure() instead. - * - * @see https://www.drupal.org/node/2940126 */ function file_ensure_htaccess() { - @trigger_error("file_ensure_htaccess() is deprecated in Drupal 8.7.0 and will be removed before Drupal 9.0.0. Use \Drupal\Core\File\HtaccessWriter::ensure() instead. See https://www.drupal.org/node/2940126", E_USER_DEPRECATED); - \Drupal::service('file.htaccess_writer')->ensure(); + file_save_htaccess('public://', FALSE); + $private_path = PrivateStream::basePath(); + if (!empty($private_path)) { + file_save_htaccess('private://', TRUE); + } + file_save_htaccess('temporary://', TRUE); + + // If a staging directory exists then it should contain a .htaccess file. + // @todo https://www.drupal.org/node/2696103 catch a more specific exception + // and simplify this code. + try { + $staging = config_get_config_directory(CONFIG_SYNC_DIRECTORY); + } + catch (\Exception $e) { + $staging = FALSE; + } + if ($staging) { + // Note that we log an error here if we can't write the .htaccess file. This + // can occur if the staging directory is read-only. If it is then it is the + // user's responsibility to create the .htaccess file. + file_save_htaccess($staging, TRUE); + } } /** @@ -336,18 +352,31 @@ function file_ensure_htaccess() { * @param bool $force_overwrite * (Optional) Set to TRUE to attempt to overwrite the existing .htaccess file * if one is already present. Defaults to FALSE. - * - * @return bool - * TRUE when file exists or created successfully, FALSE otherwise. - * - * @deprecated in Drupal 8.7.x, will be removed before Drupal 9.0.0. Use - * \Drupal\Core\File\HtaccessWriterInterface::save() instead. - * - * @see https://www.drupal.org/node/2940126 */ function file_save_htaccess($directory, $private = TRUE, $force_overwrite = FALSE) { - @trigger_error('file_save_htaccess() is deprecated in Drupal 8.7.0 and will be removed before Drupal 9.0.0. Use \Drupal\Core\File\HtaccessWriter::save() instead. See https://www.drupal.org/node/2940126', E_USER_DEPRECATED); - return \Drupal::service('file.htaccess_writer')->save($directory, $private, $force_overwrite); + if (\Drupal::service('file_system')->uriScheme($directory)) { + $htaccess_path = file_stream_wrapper_uri_normalize($directory . '/.htaccess'); + } + else { + $directory = rtrim($directory, '/\\'); + $htaccess_path = $directory . '/.htaccess'; + } + + if (file_exists($htaccess_path) && !$force_overwrite) { + // Short circuit if the .htaccess file already exists. + return TRUE; + } + $htaccess_lines = FileStorage::htaccessLines($private); + + // Write the .htaccess file. + if (file_exists($directory) && is_writable($directory) && file_put_contents($htaccess_path, $htaccess_lines)) { + return drupal_chmod($htaccess_path, 0444); + } + else { + $variables = ['%directory' => $directory, '@htaccess' => $htaccess_lines]; + \Drupal::logger('security')->error("Security warning: Couldn't write .htaccess file. Please create a .htaccess file in your %directory directory which contains the following lines: 
@htaccess
", $variables); + return FALSE; + } } /** @@ -531,7 +560,7 @@ function file_unmanaged_prepare($source, &$destination = NULL, $replace = FILE_E return FALSE; } // Make sure the .htaccess files are present. - \Drupal::service('file.htaccess_writer')->ensure(); + file_ensure_htaccess(); return TRUE; } diff --git a/core/includes/install.core.inc b/core/includes/install.core.inc index 9ee8b5b9f82..6c8a0a7e4ec 100644 --- a/core/includes/install.core.inc +++ b/core/includes/install.core.inc @@ -1120,11 +1120,14 @@ function install_base_system(&$install_state) { ->save(); } - // Ensure that all of Drupal's standard directories (e.g., the public files - // directory and config directory) have appropriate .htaccess files. These - // directories will have already been created by this point in the installer, - // since Drupal creates them during the install_verify_requirements() task. - \Drupal::service("file.htaccess_writer")->ensure(); + // Call file_ensure_htaccess() to ensure that all of Drupal's standard + // directories (e.g., the public files directory and config directory) have + // appropriate .htaccess files. These directories will have already been + // created by this point in the installer, since Drupal creates them during + // the install_verify_requirements() task. Note that we cannot call + // file_ensure_access() any earlier than this, since it relies on + // system.module in order to work. + file_ensure_htaccess(); // Prime the drupal_get_filename() static cache with the user module's // exact location. diff --git a/core/lib/Drupal/Core/Config/FileStorage.php b/core/lib/Drupal/Core/Config/FileStorage.php index fe145403357..60c5fa11d65 100644 --- a/core/lib/Drupal/Core/Config/FileStorage.php +++ b/core/lib/Drupal/Core/Config/FileStorage.php @@ -79,7 +79,7 @@ class FileStorage implements StorageInterface { $success = file_prepare_directory($dir, FILE_CREATE_DIRECTORY | FILE_MODIFY_PERMISSIONS); // Only create .htaccess file in root directory. if ($dir == $this->directory) { - $success = $success && $this->createHtaccess($this->directory); + $success = $success && file_save_htaccess($this->directory, TRUE, TRUE); } if (!$success) { throw new StorageException('Failed to create config directory ' . $dir); @@ -87,20 +87,6 @@ class FileStorage implements StorageInterface { return $this; } - /** - * Creates a .htaccess file in the given directory. - * - * @param string $directory - * The directory. - * - * @return bool - * TRUE if the .htaccess file was saved or already exists, FALSE otherwise. - */ - private function createHtaccess($directory) { - // @todo Properly inject services https://www.drupal.org/node/2940135 - return \Drupal::service('file.htaccess_writer')->save($directory, TRUE, TRUE); - } - /** * {@inheritdoc} */ diff --git a/core/lib/Drupal/Core/File/HtaccessWriter.php b/core/lib/Drupal/Core/File/HtaccessWriter.php deleted file mode 100644 index 89731a9fef0..00000000000 --- a/core/lib/Drupal/Core/File/HtaccessWriter.php +++ /dev/null @@ -1,143 +0,0 @@ -fileSystem = $file_system; - $this->logger = $logger; - } - - /** - * {@inheritdoc} - */ - public function ensure() { - try { - foreach ($this->getHtaccessFiles() as $htaccessFile => $info) { - $this->save($info['directory'], $info['private']); - } - - // If a staging directory exists then it should contain a .htaccess file. - // @todo https://www.drupal.org/node/2696103 catch a more specific - // exception and simplify this code. - try { - $staging = config_get_config_directory(CONFIG_SYNC_DIRECTORY); - } - catch (\Exception $e) { - $staging = FALSE; - } - if ($staging) { - // Note that we log an error here if we can't write the .htaccess file. - // This can occur if the staging directory is read-only. If it is then - // it is the user's responsibility to create the .htaccess file. - $this->save($staging, TRUE); - } - } - catch (\Exception $e) { - $this->logger->error($e->getMessage()); - } - } - - /** - * {@inheritdoc} - */ - public function save($directory, $private = TRUE, $forceOverwrite = FALSE) { - if ($this->fileSystem->uriScheme($directory)) { - $htaccessPath = $this->normalizeUri($directory . '/.htaccess'); - } - else { - $directory = rtrim($directory, '/\\'); - $htaccessPath = $directory . '/.htaccess'; - } - - if (file_exists($htaccessPath) && !$forceOverwrite) { - // Short circuit if the .htaccess file already exists. - return TRUE; - } - $htaccessLines = FileStorage::htaccessLines($private); - - // Write the .htaccess file. - if (file_exists($directory) && is_writable($directory) && file_put_contents($htaccessPath, $htaccessLines)) { - return $this->fileSystem->chmod($htaccessPath, 0444); - } - else { - $this->logger->error("Security warning: Couldn't write .htaccess file. Please create a .htaccess file in your %directory directory which contains the following lines: 
@htaccess
", [ - '%directory' => $directory, - '@htaccess' => $htaccessLines, - ]); - return FALSE; - } - } - - /** - * {@inheritdoc} - */ - public function getHtaccessFiles() { - $htaccessFiles = []; - $htaccessFiles['public://.htaccess'] = [ - 'title' => new TranslatableMarkup('Public files directory'), - 'directory' => $this->fileSystem->realpath('public://'), - 'private' => FALSE, - ]; - if (PrivateStream::basePath()) { - $htaccessFiles['private://.htaccess'] = [ - 'title' => new TranslatableMarkup('Private files directory'), - 'directory' => $this->fileSystem->realpath('private://'), - 'private' => TRUE, - ]; - } - $htaccessFiles['temporary://.htaccess'] = [ - 'title' => new TranslatableMarkup('Temporary files directory'), - 'directory' => $this->fileSystem->realpath('temporary://'), - 'private' => TRUE, - ]; - return $htaccessFiles; - } - - /** - * Wraps file_stream_wrapper_uri_normalize(). - * - * @param string $uri - * A URI to normalize. - * - * @return string - * The normalized URI. - * - * @see file_stream_wrapper_uri_normalize() - */ - private function normalizeUri($uri) { - return file_stream_wrapper_uri_normalize($uri); - } - -} diff --git a/core/lib/Drupal/Core/File/HtaccessWriterInterface.php b/core/lib/Drupal/Core/File/HtaccessWriterInterface.php deleted file mode 100644 index a6385cf98be..00000000000 --- a/core/lib/Drupal/Core/File/HtaccessWriterInterface.php +++ /dev/null @@ -1,44 +0,0 @@ -save(\Drupal::root() . '/' . $site_directory, FALSE)) { + elseif (!file_save_htaccess(\Drupal::root() . '/' . $site_directory, FALSE)) { $requirements['simpletest_site_directory'] = [ 'title' => t('Simpletest site directory'), 'value' => t('Not protected'), diff --git a/core/modules/system/system.install b/core/modules/system/system.install index e54a3190fa7..e54797bb45f 100644 --- a/core/modules/system/system.install +++ b/core/modules/system/system.install @@ -490,10 +490,23 @@ function system_requirements($phase) { if ($phase == 'runtime') { // Try to write the .htaccess files first, to prevent false alarms in case // (for example) the /tmp directory was wiped. - /** @var \Drupal\Core\File\HtaccessWriterInterface $htaccessWriter */ - $htaccessWriter = \Drupal::service("file.htaccess_writer"); - $htaccessWriter->ensure(); - foreach ($htaccessWriter->getHtaccessFiles() as $htaccess_file => $info) { + file_ensure_htaccess(); + $file_system = \Drupal::service('file_system'); + $htaccess_files['public://.htaccess'] = [ + 'title' => t('Public files directory'), + 'directory' => $file_system->realpath('public://'), + ]; + if (PrivateStream::basePath()) { + $htaccess_files['private://.htaccess'] = [ + 'title' => t('Private files directory'), + 'directory' => $file_system->realpath('private://'), + ]; + } + $htaccess_files['temporary://.htaccess'] = [ + 'title' => t('Temporary files directory'), + 'directory' => $file_system->realpath('temporary://'), + ]; + foreach ($htaccess_files as $htaccess_file => $info) { // Check for the string which was added to the recommended .htaccess file // in the latest security update. if (!file_exists($htaccess_file) || !($contents = @file_get_contents($htaccess_file)) || strpos($contents, 'Drupal_Security_Do_Not_Remove_See_SA_2013_003') === FALSE) { diff --git a/core/modules/system/system.module b/core/modules/system/system.module index b1fa5631b73..13456c762dc 100644 --- a/core/modules/system/system.module +++ b/core/modules/system/system.module @@ -929,15 +929,13 @@ function system_check_directory($form_element, FormStateInterface $form_state) { $logger->error('The directory %directory exists but is not writable and could not be made writable.', ['%directory' => $directory]); } elseif (is_dir($directory)) { - /** @var \Drupal\Core\File\HtaccessWriterInterface $htaccess */ - $htaccess = \Drupal::service('file.htaccess_writer'); if ($form_element['#name'] == 'file_public_path') { // Create public .htaccess file. - $htaccess->save($directory, FALSE); + file_save_htaccess($directory, FALSE); } else { // Create private .htaccess file. - $htaccess->save($directory); + file_save_htaccess($directory); } } diff --git a/core/modules/system/tests/src/Functional/File/FileSaveHtaccessLoggingTest.php b/core/modules/system/tests/src/Functional/File/FileSaveHtaccessLoggingTest.php index 96cab90f8c4..003373de718 100644 --- a/core/modules/system/tests/src/Functional/File/FileSaveHtaccessLoggingTest.php +++ b/core/modules/system/tests/src/Functional/File/FileSaveHtaccessLoggingTest.php @@ -24,16 +24,14 @@ class FileSaveHtaccessLoggingTest extends BrowserTestBase { // Verify that file_save_htaccess() returns FALSE if .htaccess cannot be // written and writes a correctly formatted message to the error log. Set // $private to TRUE so all possible .htaccess lines are written. - /** @var \Drupal\Core\File\HtaccessWriterInterface $htaccess */ - $htaccess = \Drupal::service('file.htaccess_writer'); - $this->assertFalse($htaccess->save($private, TRUE)); + $this->assertFalse(file_save_htaccess($private, TRUE)); $this->drupalLogin($this->rootUser); $this->drupalGet('admin/reports/dblog'); $this->clickLink("Security warning: Couldn't write .htaccess file. Please…"); $lines = FileStorage::htaccessLines(TRUE); foreach (array_filter(explode("\n", $lines)) as $line) { - $this->assertSession()->assertEscaped($line); + $this->assertEscaped($line); } } diff --git a/core/tests/Drupal/KernelTests/Core/File/DirectoryTest.php b/core/tests/Drupal/KernelTests/Core/File/DirectoryTest.php index 2b15d082c7e..467d9262e16 100644 --- a/core/tests/Drupal/KernelTests/Core/File/DirectoryTest.php +++ b/core/tests/Drupal/KernelTests/Core/File/DirectoryTest.php @@ -88,7 +88,7 @@ class DirectoryTest extends FileTestBase { // Remove .htaccess file to then test that it gets re-created. @drupal_unlink(file_default_scheme() . '://.htaccess'); $this->assertFalse(is_file(file_default_scheme() . '://.htaccess'), 'Successfully removed the .htaccess file in the files directory.', 'File'); - $this->container->get("file.htaccess_writer")->ensure(); + file_ensure_htaccess(); $this->assertTrue(is_file(file_default_scheme() . '://.htaccess'), 'Successfully re-created the .htaccess file in the files directory.', 'File'); // Verify contents of .htaccess file. $file = file_get_contents(file_default_scheme() . '://.htaccess'); diff --git a/core/tests/Drupal/KernelTests/Core/File/HtaccessDeprecationTest.php b/core/tests/Drupal/KernelTests/Core/File/HtaccessDeprecationTest.php deleted file mode 100644 index fd0422cd998..00000000000 --- a/core/tests/Drupal/KernelTests/Core/File/HtaccessDeprecationTest.php +++ /dev/null @@ -1,28 +0,0 @@ -public = Settings::get('file_public_path') . '/test/public'; - $this->htaccess = $this->container->get('file.htaccess_writer'); - } - - /** - * @covers ::save + * Tests file_save_htaccess(). */ public function testHtaccessSave() { // Prepare test directories. + $public = Settings::get('file_public_path') . '/test/public'; $private = Settings::get('file_public_path') . '/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($this->public, 0777, TRUE); - $this->assertTrue($this->htaccess->save($this->public, FALSE)); - $content = file_get_contents($this->public . '/.htaccess'); + mkdir($public, 0777, TRUE); + $this->assertTrue(file_save_htaccess($public, FALSE)); + $content = file_get_contents($public . '/.htaccess'); $this->assertTrue(strpos($content, "SetHandler Drupal_Security_Do_Not_Remove_See_SA_2006_006") !== FALSE); $this->assertFalse(strpos($content, "Require all denied") !== FALSE); $this->assertFalse(strpos($content, "Deny from all") !== FALSE); $this->assertTrue(strpos($content, "Options -Indexes -ExecCGI -Includes -MultiViews") !== FALSE); $this->assertTrue(strpos($content, "SetHandler Drupal_Security_Do_Not_Remove_See_SA_2013_003") !== FALSE); - $this->assertFilePermissions($this->public . '/.htaccess', 0444); + $this->assertFilePermissions($public . '/.htaccess', 0444); - $this->assertTrue($this->htaccess->save($this->public, FALSE)); + $this->assertTrue(file_save_htaccess($public, FALSE)); // Create private .htaccess file. mkdir($private, 0777, TRUE); - $this->assertTrue($this->htaccess->save($private)); + $this->assertTrue(file_save_htaccess($private)); $content = file_get_contents($private . '/.htaccess'); $this->assertTrue(strpos($content, "SetHandler Drupal_Security_Do_Not_Remove_See_SA_2006_006") !== FALSE); $this->assertTrue(strpos($content, "Require all denied") !== FALSE); @@ -74,11 +53,11 @@ class HtaccessTest extends KernelTestBase { $this->assertTrue(strpos($content, "SetHandler Drupal_Security_Do_Not_Remove_See_SA_2013_003") !== FALSE); $this->assertFilePermissions($private . '/.htaccess', 0444); - $this->assertTrue($this->htaccess->save($private)); + $this->assertTrue(file_save_htaccess($private)); // Create an .htaccess file using a stream URI. mkdir($stream, 0777, TRUE); - $this->assertTrue($this->htaccess->save($stream)); + $this->assertTrue(file_save_htaccess($stream)); $content = file_get_contents($stream . '/.htaccess'); $this->assertTrue(strpos($content, "SetHandler Drupal_Security_Do_Not_Remove_See_SA_2006_006") !== FALSE); $this->assertTrue(strpos($content, "Require all denied") !== FALSE); @@ -87,7 +66,7 @@ class HtaccessTest extends KernelTestBase { $this->assertTrue(strpos($content, "SetHandler Drupal_Security_Do_Not_Remove_See_SA_2013_003") !== FALSE); $this->assertFilePermissions($stream . '/.htaccess', 0444); - $this->assertTrue($this->htaccess->save($stream)); + $this->assertTrue(file_save_htaccess($stream)); } /** @@ -97,10 +76,13 @@ class HtaccessTest extends KernelTestBase { * 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; - $this->assertSame($actual, $expected, new FormattableMarkup('@uri file permissions @actual are identical to @expected.', [ + return $this->assertIdentical($actual, $expected, new FormattableMarkup('@uri file permissions @actual are identical to @expected.', [ '@uri' => $uri, '@actual' => 0 . decoct($actual), '@expected' => 0 . decoct($expected),