diff --git a/core/lib/Drupal/Core/File/FileSystem.php b/core/lib/Drupal/Core/File/FileSystem.php index 2f30ed030351859bdde432891b2aebbf2763048e..d2e1e3d977a11a26a54852a596ed20a5b6c5f45b 100644 --- a/core/lib/Drupal/Core/File/FileSystem.php +++ b/core/lib/Drupal/Core/File/FileSystem.php @@ -2,6 +2,7 @@ namespace Drupal\Core\File; +use Drupal\Component\Utility\Unicode; use Drupal\Core\File\Exception\DirectoryNotReadyException; use Drupal\Core\File\Exception\FileException; use Drupal\Core\File\Exception\FileExistsException; @@ -571,6 +572,10 @@ public function prepareDirectory(&$directory, $options = self::MODIFY_PERMISSION * {@inheritdoc} */ public function getDestinationFilename($destination, $replace) { + $basename = $this->basename($destination); + if (!Unicode::validateUtf8($basename)) { + throw new FileException(sprintf("Invalid filename '%s'", $basename)); + } if (file_exists($destination)) { switch ($replace) { case FileSystemInterface::EXISTS_REPLACE: @@ -578,7 +583,6 @@ public function getDestinationFilename($destination, $replace) { break; case FileSystemInterface::EXISTS_RENAME: - $basename = $this->basename($destination); $directory = $this->dirname($destination); $destination = $this->createFilename($basename, $directory); break; @@ -595,9 +599,13 @@ public function getDestinationFilename($destination, $replace) { * {@inheritdoc} */ public function createFilename($basename, $directory) { + $original = $basename; // 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 (preg_last_error() !== PREG_NO_ERROR) { + throw new FileException(sprintf("Invalid filename '%s'", $original)); + } if (substr(PHP_OS, 0, 3) == 'WIN') { // These characters are not allowed in Windows filenames. $basename = str_replace([':', '*', '?', '"', '<', '>', '|'], '_', $basename); diff --git a/core/lib/Drupal/Core/File/FileSystemInterface.php b/core/lib/Drupal/Core/File/FileSystemInterface.php index c35ff2c26eca2d3e8d29499d61c4eefc83600bb7..262666d92a6715e05a411aff29d813d67d4bb182 100644 --- a/core/lib/Drupal/Core/File/FileSystemInterface.php +++ b/core/lib/Drupal/Core/File/FileSystemInterface.php @@ -439,6 +439,9 @@ public function prepareDirectory(&$directory, $options = self::MODIFY_PERMISSION * @return string * File path consisting of $directory and a unique filename based off * of $basename. + * + * @throws \Drupal\Core\File\Exception\FileException + * Implementation may throw FileException or its subtype on failure. */ public function createFilename($basename, $directory); @@ -457,6 +460,9 @@ public function createFilename($basename, $directory); * @return string|bool * The destination filepath, or FALSE if the file already exists * and FileSystemInterface::EXISTS_ERROR is specified. + * + * @throws \Drupal\Core\File\Exception\FileException + * Implementation may throw FileException or its subtype on failure. */ public function getDestinationFilename($destination, $replace); diff --git a/core/modules/file/file.module b/core/modules/file/file.module index 6e812bca744c4a2b309bf9241a36bd229c574c5c..809cc3b1291e36e4e94e4c4136b334fedc09c753 100644 --- a/core/modules/file/file.module +++ b/core/modules/file/file.module @@ -1038,7 +1038,13 @@ function _file_save_upload_single(\SplFileInfo $file_info, $form_field_name, $va } /** @var \Drupal\Core\File\FileSystemInterface $file_system */ $file_system = \Drupal::service('file_system'); - $file->destination = $file_system->getDestinationFilename($destination . $file->getFilename(), $replace); + try { + $file->destination = $file_system->getDestinationFilename($destination . $file->getFilename(), $replace); + } + catch (FileException $e) { + \Drupal::messenger()->addError(t('The file %filename could not be uploaded because the name is invalid.', ['%filename' => $file->getFilename()])); + return FALSE; + } // If the destination is FALSE then there is an existing file and $replace is // set to return an error, so we need to exit. if ($file->destination === FALSE) { diff --git a/core/modules/file/tests/src/Functional/SaveUploadTest.php b/core/modules/file/tests/src/Functional/SaveUploadTest.php index d8e66b7e177745a682cad243605ad065cf599719..8d5f01a43688176045f8bfaba2681bb93cd71cc3 100644 --- a/core/modules/file/tests/src/Functional/SaveUploadTest.php +++ b/core/modules/file/tests/src/Functional/SaveUploadTest.php @@ -2,7 +2,9 @@ namespace Drupal\Tests\file\Functional; +use Drupal\Component\Render\FormattableMarkup; use Drupal\Core\File\FileSystemInterface; +use Drupal\Core\Url; use Drupal\file\Entity\File; use Drupal\Tests\TestFileCreationTrait; @@ -371,4 +373,60 @@ public function testDrupalMovingUploadedFileError() { ]), 'Found upload error log entry.'); } + /** + * Tests that filenames containing invalid UTF-8 are rejected. + */ + public function testInvalidUtf8FilenameUpload() { + $this->drupalGet('file-test/upload'); + + // Filename containing invalid UTF-8. + $filename = "x\xc0xx.gif"; + + $page = $this->getSession()->getPage(); + $data = [ + 'multipart' => [ + [ + 'name' => 'file_test_replace', + 'contents' => FileSystemInterface::EXISTS_RENAME, + ], + [ + 'name' => 'form_id', + 'contents' => '_file_test_form', + ], + [ + 'name' => 'form_build_id', + 'contents' => $page->find('hidden_field_selector', ['hidden_field', 'form_build_id'])->getAttribute('value'), + ], + [ + 'name' => 'form_token', + 'contents' => $page->find('hidden_field_selector', ['hidden_field', 'form_token'])->getAttribute('value'), + ], + [ + 'name' => 'op', + 'contents' => 'Submit', + ], + [ + 'name' => 'files[file_test_upload]', + 'contents' => 'Test content', + 'filename' => $filename, + ], + ], + 'cookies' => $this->getSessionCookies(), + 'http_errors' => FALSE, + ]; + + $this->assertFileNotExists('temporary://' . $filename); + // Use Guzzle's HTTP client directly so we can POST files without having to + // write them to disk. Not all filesystem support writing files with invalid + // UTF-8 filenames. + $response = $this->getHttpClient()->request('POST', Url::fromUri('base:file-test/upload')->setAbsolute()->toString(), $data); + + $content = (string) $response->getBody(); + $this->htmlOutput($content); + $error_text = new FormattableMarkup('The file %filename could not be uploaded because the name is invalid.', ['%filename' => $filename]); + $this->assertContains((string) $error_text, $content); + $this->assertContains('Epic upload FAIL!', $content); + $this->assertFileNotExists('temporary://' . $filename); + } + } diff --git a/core/tests/Drupal/KernelTests/Core/File/DirectoryTest.php b/core/tests/Drupal/KernelTests/Core/File/DirectoryTest.php index 5abfb060bfe8b510462fe01f5fb26b32b8795ba8..ca2d873cd8752bc572116560d23c23411eaf512c 100644 --- a/core/tests/Drupal/KernelTests/Core/File/DirectoryTest.php +++ b/core/tests/Drupal/KernelTests/Core/File/DirectoryTest.php @@ -3,6 +3,7 @@ namespace Drupal\KernelTests\Core\File; use Drupal\Component\PhpStorage\FileStorage; +use Drupal\Core\File\Exception\FileException; use Drupal\Core\File\FileSystemInterface; /** @@ -156,6 +157,10 @@ public function testFileDestination() { $this->assertNotEqual($path, $destination, 'A new filepath destination is created when filepath destination already exists with FileSystemInterface::EXISTS_RENAME.', 'File'); $path = $file_system->getDestinationFilename($destination, FileSystemInterface::EXISTS_ERROR); $this->assertEqual($path, FALSE, 'An error is returned when filepath destination already exists with FileSystemInterface::EXISTS_ERROR.', 'File'); + + // Invalid UTF-8 causes an exception. + $this->setExpectedException(FileException::class, "Invalid filename 'a\xFFtest\x80€.txt'"); + $file_system->getDestinationFilename("core/misc/a\xFFtest\x80€.txt", FileSystemInterface::EXISTS_REPLACE); } /** diff --git a/core/tests/Drupal/Tests/Core/File/FileSystemTest.php b/core/tests/Drupal/Tests/Core/File/FileSystemTest.php index 247e9f1b05c97571afc8b6fa0c4b210dd39acdde..9392b7254a52ca73c48aac6abf5b619eb721ded9 100644 --- a/core/tests/Drupal/Tests/Core/File/FileSystemTest.php +++ b/core/tests/Drupal/Tests/Core/File/FileSystemTest.php @@ -2,6 +2,7 @@ namespace Drupal\Tests\Core\File; +use Drupal\Core\File\Exception\FileException; use Drupal\Core\File\FileSystem; use Drupal\Core\Site\Settings; use Drupal\Tests\UnitTestCase; @@ -175,4 +176,16 @@ protected function assertFilePermissions($expected_mode, $uri, $message = '') { $this->assertSame($expected_mode, $actual_mode, $message); } + /** + * Tests that invalid UTF-8 results in an exception. + * + * @covers ::createFilename + */ + public function testInvalidUTF8() { + vfsStream::setup('dir'); + $filename = "a\xFFsdf\x80€" . '.txt'; + $this->setExpectedException(FileException::class, "Invalid filename '$filename'"); + $this->fileSystem->createFilename($filename, 'vfs://dir'); + } + }