fix: #3562543 FileEventSubscriber::sanitizeFilename() does not check for invalid UTF-8 chars (482f3cee) · Commits · project / drupal

core/modules/file/src/EventSubscriber/FileEventSubscriber.php

+53 −2

Changes for core/modules/file/src/EventSubscriber/FileEventSubscriber.php: 53 added lines, 2 removed lines.
Original line number	Diff line number	Diff line
		@@ -4,9 +4,9 @@

		use Drupal\Component\Transliteration\TransliterationInterface;
		use Drupal\Core\Config\ConfigFactoryInterface;
		use Drupal\Core\File\Event\FileUploadSanitizeNameEvent;
		use Drupal\Core\Language\LanguageInterface;
		use Drupal\Core\Language\LanguageManagerInterface;
		use Drupal\Core\File\Event\FileUploadSanitizeNameEvent;
		use Symfony\Component\DependencyInjection\Attribute\Autowire;
		use Symfony\Component\EventDispatcher\EventSubscriberInterface;

		@@ -39,10 +39,31 @@ public function __construct(
		*/
		public static function getSubscribedEvents(): array {
		return [
		FileUploadSanitizeNameEvent::class => 'sanitizeFilename',
		FileUploadSanitizeNameEvent::class => [
		// Run before every other listener so that they can all rely on the
		// filename being valid UTF-8, and therefore safe to manipulate with
		// PCRE's /u modifier and the mb_* functions.
		['ensureValidUtf8Filename', PHP_INT_MAX],
		['sanitizeFilename'],
		],
		];
		}

		/**
		* Guarantees a valid UTF-8 filename for subsequent event handlers.
		*
		* @param \Drupal\Core\File\Event\FileUploadSanitizeNameEvent $event
		* File upload sanitize name event.
		*/
		public function ensureValidUtf8Filename(FileUploadSanitizeNameEvent $event): void {
		$filename = $event->getFilename();
		if (!mb_check_encoding($filename, 'UTF-8')) {
		$replacement = $this->configFactory->get('file.settings')
		->get('filename_sanitization.replacement_character');
		$event->setFilename(self::coerceToValidUtf8($filename, $replacement));
		}
		}

		/**
		* Sanitizes the filename of a file being uploaded.
		*
		@@ -81,6 +102,8 @@ public function sanitizeFilename(FileUploadSanitizeNameEvent $event) {
		$alphanumeric = TRUE;
		}
		}

		// ::ensureValidUtf8Filename() ensures the /u modifier is safe here.
		if ($fileSettings->get('filename_sanitization.replace_whitespace')) {
		$filename = preg_replace('/\s/u', $replacement, trim($filename));
		}
		@@ -109,4 +132,32 @@ public function sanitizeFilename(FileUploadSanitizeNameEvent $event) {
		$event->setFilename($filename . $extension);
		}

		/**
		* Replaces invalid UTF-8 byte sequences in a string.
		*
		* PHP's mb_convert_encoding() replaces every invalid byte with a ? so any
		* that already present are preserved when a different $unknown_character is
		* specified.
		*
		* @param string $string
		* The string to coerce to valid UTF-8.
		* @param string $unknown_character
		* The character substituted for invalid byte sequences.
		*
		* @return string
		* The string as valid UTF-8.
		*/
		private static function coerceToValidUtf8(string $string, string $unknown_character = '?'): string {
		$code_point = mb_ord($unknown_character, 'UTF-8');
		if ($code_point === FALSE) {
		throw new \InvalidArgumentException('$unknown_character must be a single valid UTF-8 character.');
		}

		$previous = mb_substitute_character();
		mb_substitute_character($code_point);
		$string = mb_convert_encoding($string, 'UTF-8', 'UTF-8');
		mb_substitute_character($previous);
		return $string;
		}

		}

core/modules/file/tests/src/Functional/SaveUploadTest.php

+7 −5

Changes for core/modules/file/tests/src/Functional/SaveUploadTest.php: 7 added lines, 5 removed lines.
Original line number	Diff line number	Diff line
		@@ -4,7 +4,6 @@

		namespace Drupal\Tests\file\Functional;

		use Drupal\Component\Utility\Html;
		use Drupal\Core\Database\Database;
		use Drupal\Core\File\FileExists;
		use Drupal\Core\Url;
		@@ -709,7 +708,7 @@ public function testDrupalMovingUploadedFileError(): void {
		}

		/**
		* Tests that filenames containing invalid UTF-8 are rejected.
		* Tests that invalid UTF-8 in a filename is replaced rather than rejected.
		*/
		public function testInvalidUtf8FilenameUpload(): void {
		$this->drupalGet('file-test/upload');
		@@ -758,9 +757,12 @@ public function testInvalidUtf8FilenameUpload(): void {

		$content = (string) $response->getBody();
		$this->htmlOutput($content);
		$error_text = 'The file <em class="placeholder">' . Html::escape($filename) . '</em> could not be uploaded because the name is invalid.';
		$this->assertStringContainsString($error_text, $content);
		$this->assertStringContainsString('Epic upload FAIL!', $content);
		// The invalid byte is replaced with the configured replacement character,
		// so the upload succeeds rather than failing with a confusing message.
		$this->assertStringContainsString('You WIN!', $content);
		$this->assertStringContainsString('File name is x-xx.gif.', $content);
		$this->assertStringNotContainsString('Epic upload FAIL!', $content);
		$this->assertFileExists('temporary://x-xx.gif');
		$this->assertFileDoesNotExist('temporary://' . $filename);
		}

core/modules/file/tests/src/Unit/SanitizeNameTest.php

+104 −0

Changes for core/modules/file/tests/src/Unit/SanitizeNameTest.php: 104 added lines, 0 removed lines.
Original line number	Diff line number	Diff line
		@@ -69,6 +69,54 @@ public function testFileNameTransliteration($original, $expected, array $options
		$this->assertEquals($expected, $event->getFilename());
		}

		/**
		* Tests that filenames with invalid UTF-8 are coerced, not destroyed.
		*
		* Both listeners are invoked, in the order the event dispatcher runs them,
		* because ::ensureValidUtf8Filename() is what makes the filename safe for
		* the PCRE /u modifier used throughout ::sanitizeFilename().
		*
		* @param string $original
		* The original filename.
		* @param string $expected
		* The expected filename.
		* @param array $options
		* Array of filename sanitization options, in this order:
		* 0: boolean Transliterate.
		* 1: string Character to use in replacements.
		* 2: boolean Replace whitespace.
		* 3: boolean Replace non-alphanumeric characters.
		* 4: boolean De-duplicate separators.
		* 5: boolean Convert to lowercase.
		*/
		#[DataProvider('provideInvalidUtf8Filenames')]
		public function testInvalidUtf8Filename(string $original, string $expected, array $options): void {
		$config_factory = $this->getConfigFactoryStub([
		'file.settings' => [
		'filename_sanitization' => [
		'transliterate' => $options[0],
		'replacement_character' => $options[1],
		'replace_whitespace' => $options[2],
		'replace_non_alphanumeric' => $options[3],
		'deduplicate_separators' => $options[4],
		'lowercase' => $options[5],
		],
		],
		]);
		$language_manager = $this->prophesize(LanguageManagerInterface::class);
		$language_manager->getCurrentLanguage(LanguageInterface::TYPE_CONTENT)
		->willReturn(new Language(['id' => 'en']));

		$event = new FileUploadSanitizeNameEvent($original, 'en');
		$subscriber = new FileEventSubscriber($config_factory, new PhpTransliteration(), $language_manager->reveal());
		$subscriber->ensureValidUtf8Filename($event);
		$subscriber->sanitizeFilename($event);

		$result = $event->getFilename();
		$this->assertTrue(mb_check_encoding($result, 'UTF-8'), 'Sanitized filename is valid UTF-8.');
		$this->assertSame($expected, $result);
		}

		/**
		* Provides data for testFileNameTransliteration().
		*
		@@ -245,4 +293,60 @@ public static function provideFilenames() {
		];
		}

		/**
		* Provides data for testInvalidUtf8Filename().
		*
		* @return array
		* Arrays with original name, expected name, and sanitization options.
		*/
		public static function provideInvalidUtf8Filenames(): array {
		return [
		'No transliteration: replace (-)' => [
		"bad\xFFname.txt",
		'bad-name.txt',
		[FALSE, '-', TRUE, FALSE, FALSE, FALSE],
		],
		'No transliteration: replace (_)' => [
		"bad\xFFname.txt",
		'bad_name.txt',
		[FALSE, '_', TRUE, FALSE, FALSE, FALSE],
		],
		'No transliteration: raw file without extension' => [
		"bad\xFFname",
		'bad-name',
		[FALSE, '-', TRUE, FALSE, FALSE, FALSE],
		],
		'No transliteration: every byte invalid' => [
		"\xFF\xFE\xFD.txt",
		'---.txt',
		[FALSE, '-', TRUE, FALSE, FALSE, FALSE],
		],
		'No transliteration: existing question mark is preserved' => [
		"why?\xFFnot.txt",
		'why?-not.txt',
		[FALSE, '-', TRUE, FALSE, FALSE, FALSE],
		],
		'Transliteration: replace (-)' => [
		"Á-TÉXT-\xFFœ.txt",
		'A-TEXT--oe.txt',
		[TRUE, '-', FALSE, FALSE, FALSE, FALSE],
		],
		'Transliteration: replace (_)' => [
		"Á-TÉXT-\xFFœ.txt",
		'A-TEXT-_oe.txt',
		[TRUE, '_', FALSE, FALSE, FALSE, FALSE],
		],
		'Transliteration: existing question mark is preserved' => [
		"why?\xFFnot.txt",
		'why?-not.txt',
		[TRUE, '-', FALSE, FALSE, FALSE, FALSE],
		],
		'Transliteration and replace whitespace: complex' => [
		"S Pácê\xFF--táb#\t#--🙈.jpg",
		'S--Pace---tab#-#---.jpg',
		[TRUE, '-', TRUE, FALSE, FALSE, FALSE],
		],
		];
		}

		}