diff --git a/core/includes/file.inc b/core/includes/file.inc index 27e6eb6..d992b88 100644 --- a/core/includes/file.inc +++ b/core/includes/file.inc @@ -702,8 +702,14 @@ function file_unmanaged_move($source, $destination = NULL, $replace = FILE_EXIST * * @return string * The potentially modified $filename. + * + * @deprecated in Drupal 8.7.0 and will be removed before Drupal 9.0.0. Dispatch + * a \Drupal\Core\File\Event\FileUploadSanitizeNameEvent event instead. + * + * @see https://www.drupal.org/node/3032541 */ function file_munge_filename($filename, $extensions, $alerts = TRUE) { + @trigger_error('file_munge_filename() is deprecated in Drupal 8.7.0 and will be removed before Drupal 9.0.0. Dispatch a \Drupal\Core\File\Event\FileEvents::SANITIZE event instead. See https://www.drupal.org/node/3032541', E_USER_DEPRECATED); $original = $filename; // Allow potentially insecure uploads for very savvy users and admin @@ -749,8 +755,14 @@ function file_munge_filename($filename, $extensions, $alerts = TRUE) { * * @return * An unmunged filename string. + * + * @deprecated in Drupal 8.7.0 and will be removed before Drupal 9.0.0. Use + * str_replace() instead. + * + * @see https://www.drupal.org/node/3032541 */ function file_unmunge_filename($filename) { + @trigger_error('file_unmunge_filename() is deprecated in Drupal 8.7.0 and will be removed before Drupal 9.0.0. Use str_replace() instead. See https://www.drupal.org/node/3032541', E_USER_DEPRECATED); return str_replace('_.', '.', $filename); } diff --git a/core/lib/Drupal/Core/File/Event/ExtensionLockedException.php b/core/lib/Drupal/Core/File/Event/ExtensionLockedException.php new file mode 100644 index 0000000..6495f46 --- /dev/null +++ b/core/lib/Drupal/Core/File/Event/ExtensionLockedException.php @@ -0,0 +1,13 @@ +extension = pathinfo($filename, PATHINFO_EXTENSION); + if ($this->extension !== '') { + // Remove the extension using preg_replace() to prevent issues with + // multi-byte characters in filenames due to + // https://bugs.php.net/bug.php?id=77239. + $this->filename = preg_replace('@' . preg_quote('.' . $this->extension, '@') . '$@u', '', $filename); + } + else { + $this->filename = $filename; + } + $this->allowedExtensions = $allowed_extensions; + } + + /** + * Gets the file extension. + * + * @return string + * The extension (if any) of the file being uploaded. + */ + public function getExtension() { + return $this->extension; + } + + /** + * Gets the filename (without extension). + * + * @return string + * The filename (without extension) to use. + */ + public function getFilename() { + return $this->filename; + } + + /** + * Gets the full filename with extension (if any). + * + * @return string + * The full filename to use. + */ + public function getFilenameWithExtension() { + return $this->filename . ($this->extension !== '' ? ('.' . $this->extension) : ''); + } + + /** + * Sets the filename. + * + * @param string $filename + * The filename (without extension) to use for the uploaded file. + * + * @return $this + */ + public function setFilename($filename) { + $this->filename = $filename; + return $this; + } + + /** + * Gets the list of allowed extensions + * + * @return string + * The list of allowed extensions. + */ + public function getAllowedExtensions() { + return $this->allowedExtensions; + } + + /** + * Converts the file to use the .txt extension. + * + * @return $this + */ + public function makeTextFile() { + $this->filename = $this->getFilenameWithExtension(); + // Empty means everything is allowed. + if ($this->allowedExtensions) { + $this->allowedExtensions .= ' txt'; + } + $this->extension = 'txt'; + return $this; + } + + /** + * Sets the security rename flag. + * + * @return $this + */ + public function setSecurityRename() { + $this->isSecurityRename = TRUE; + return $this; + } + + /** + * Sets the file extension. + * + * @param $extension + * The new file extension + * + * @return $this + * + * @throws \Drupal\Core\File\Event\ExtensionLockedException + */ + public function setExtension($extension) { + if ($this->extensionLocked) { + throw new ExtensionLockedException('Can not set extension since it is locked. Register your event with a priority greater than 1024.'); + } + $this->extension = $extension; + return $this; + } + + /** + * Prevents the file extension from being changed any more. + * + * @return $this + */ + public function lockExtension() { + $this->extensionLocked = TRUE; + return $this; + } + + /** + * Get the security rename flag. + * + * @return bool + * TRUE if there is a rename for security reasons. + */ + public function isSecurityRename() { + return $this->isSecurityRename; + } + +} diff --git a/core/modules/file/file.module b/core/modules/file/file.module index d220cab..389851e 100644 --- a/core/modules/file/file.module +++ b/core/modules/file/file.module @@ -14,6 +14,9 @@ use Drupal\Core\Routing\RouteMatchInterface; use Drupal\Core\Url; use Drupal\file\Entity\File; +use Drupal\Core\File\Event\FileEvents; +use Drupal\Core\File\Event\FileUploadSanitizeNameEvent; +use Drupal\system\EventSubscriber\FileEventSubscriber; use Drupal\file\FileInterface; use Drupal\Component\Utility\NestedArray; use Drupal\Component\Utility\Unicode; @@ -22,8 +25,11 @@ /** * The regex pattern used when checking for insecure file types. + * + * @deprecated in Drupal 8.7.0 and will be removed before Drupal 9.0.0. Use + * \Drupal\system\EventSubscriber\FileEventSubscriber::INSECURE_EXTENSION_REGEX. */ -define('FILE_INSECURE_EXTENSION_REGEX', '/\.(phar|php|pl|py|cgi|asp|js)(\.|$)/i'); +define('FILE_INSECURE_EXTENSION_REGEX', FileEventSubscriber::INSECURE_EXTENSION_REGEX); // Load all Field module hooks for File. require_once __DIR__ . '/file.field.inc'; @@ -949,17 +955,12 @@ function _file_save_upload_single(\SplFileInfo $file_info, $form_field_name, $va return FALSE; } - // Begin building file entity. - $values = [ - 'uid' => $user->id(), - 'status' => 0, - 'filename' => $file_info->getClientOriginalName(), - 'uri' => $file_info->getRealPath(), - 'filesize' => $file_info->getSize(), - ]; - $values['filemime'] = \Drupal::service('file.mime_type.guesser')->guess($values['filename']); - $file = File::create($values); + // Remember the original filename so we can print a message if it changes. + $original_filename = $file_info->getClientOriginalName(); + + // Build a list of allowed extensions. + // @todo it'd be great to not have this here. $extensions = ''; if (isset($validators['file_validate_extensions'])) { if (isset($validators['file_validate_extensions'][0])) { @@ -975,34 +976,13 @@ function _file_save_upload_single(\SplFileInfo $file_info, $form_field_name, $va } else { // No validator was provided, so add one using the default list. - // Build a default non-munged safe list for file_munge_filename(). + // Build a default non-munged safe list for + // \Drupal\system\EventSubscriber\FileEventSubscriber::security(). $extensions = 'jpg jpeg gif png txt doc xls pdf ppt pps odt ods odp'; $validators['file_validate_extensions'] = []; $validators['file_validate_extensions'][0] = $extensions; } - if (!empty($extensions)) { - // Munge the filename to protect against possible malicious extension - // hiding within an unknown file type (ie: filename.html.foo). - $file->setFilename(file_munge_filename($file->getFilename(), $extensions)); - } - - // Rename potentially executable files, to help prevent exploits (i.e. will - // rename filename.php.foo and filename.php to filename.php.foo.txt and - // filename.php.txt, respectively). Don't rename if 'allow_insecure_uploads' - // evaluates to TRUE. - if (!\Drupal::config('system.file')->get('allow_insecure_uploads') && preg_match(FILE_INSECURE_EXTENSION_REGEX, $file->getFilename()) && (substr($file->getFilename(), -4) != '.txt')) { - $file->setMimeType('text/plain'); - // The destination filename will also later be used to create the URI. - $file->setFilename($file->getFilename() . '.txt'); - // The .txt extension may not be in the allowed list of extensions. We have - // to add it here or else the file upload will fail. - if (!empty($extensions)) { - $validators['file_validate_extensions'][0] .= ' txt'; - \Drupal::messenger()->addStatus(t('For security reasons, your upload has been renamed to %filename.', ['%filename' => $file->getFilename()])); - } - } - // If the destination is not provided, use the temporary directory. if (empty($destination)) { $destination = 'temporary://'; @@ -1015,12 +995,34 @@ function _file_save_upload_single(\SplFileInfo $file_info, $form_field_name, $va return FALSE; } - $file->source = $form_field_name; // A file URI may already have a trailing slash or look like "public://". if (substr($destination, -1) != '/') { $destination .= '/'; } - $file->destination = file_destination($destination . $file->getFilename(), $replace); + + $event = new FileUploadSanitizeNameEvent($original_filename, $extensions); + $event_dispatcher = \Drupal::service('event_dispatcher'); + $event_dispatcher->dispatch(FileEvents::SANITIZE, $event); + + if (isset($validators['file_validate_extensions'][0])) { + // @see \Drupal\Core\File\Event\FileUploadSanitizeNameEvent::makeTextFile() + $validators['file_validate_extensions'][0] = $event->getAllowedExtensions(); + } + + // Begin building file entity. + $values = [ + 'uid' => $user->id(), + 'status' => 0, + 'filename' => $file_info->getClientOriginalName(), + 'uri' => $file_info->getRealPath(), + 'filesize' => $file_info->getSize(), + ]; + $file = File::create($values); + + $file->destination = file_destination($destination . $event->getFilenameWithExtension(), $replace); + $file->setMimeType(\Drupal::service('file.mime_type.guesser')->guess($file->destination)); + $file->source = $form_field_name; + // If file_destination() returns FALSE then $replace === FILE_EXISTS_ERROR and // there's an existing file so we need to bail. if ($file->destination === FALSE) { @@ -1059,6 +1061,21 @@ function _file_save_upload_single(\SplFileInfo $file_info, $form_field_name, $va return FALSE; } + // Update the filename with any changes as a result of the FileUploadEvent + // (sanitization) or renaming due to an existing file. + $file->setFilename(\Drupal::service('file_system')->basename($file->destination)); + + // If the filename has been modified, let the user know. + if ($file->getFilename() !== $original_filename) { + if ($event->isSecurityRename()) { + $message = t('For security reasons, your upload has been renamed to %filename.', ['%filename' => $file->getFilename()]); + } + else { + $message = t('Your upload has been renamed to %filename.', ['%filename' => $file->getFilename()]); + } + \Drupal::messenger()->addStatus($message); + } + // Set the permissions on the new file. drupal_chmod($file->getFileUri()); diff --git a/core/modules/file/src/Plugin/rest/resource/FileUploadResource.php b/core/modules/file/src/Plugin/rest/resource/FileUploadResource.php index 4460036..1bb63f8 100644 --- a/core/modules/file/src/Plugin/rest/resource/FileUploadResource.php +++ b/core/modules/file/src/Plugin/rest/resource/FileUploadResource.php @@ -7,11 +7,13 @@ use Drupal\Core\Config\Config; use Drupal\Core\Entity\EntityTypeManagerInterface; use Drupal\Core\Field\FieldDefinitionInterface; +use Drupal\Core\File\Event\FileEvents; use Drupal\Core\File\FileSystemInterface; use Drupal\Core\Lock\LockBackendInterface; use Drupal\Core\Session\AccountInterface; use Drupal\Core\Utility\Token; use Drupal\file\FileInterface; +use Drupal\Core\File\Event\FileUploadSanitizeNameEvent; use Drupal\rest\ModifiedResourceResponse; use Drupal\rest\Plugin\ResourceBase; use Drupal\Component\Render\PlainTextOutput; @@ -21,6 +23,7 @@ use Drupal\rest\RequestHandler; use Psr\Log\LoggerInterface; use Symfony\Component\DependencyInjection\ContainerInterface; +use Symfony\Component\EventDispatcher\EventDispatcherInterface; use Symfony\Component\HttpFoundation\File\MimeType\MimeTypeGuesserInterface; use Symfony\Component\HttpKernel\Exception\AccessDeniedHttpException; use Symfony\Component\HttpKernel\Exception\BadRequestHttpException; @@ -125,6 +128,13 @@ class FileUploadResource extends ResourceBase { protected $systemFileConfig; /** + * The event dispatcher to dispatch the filename sanitize event. + * + * @var \Symfony\Component\EventDispatcher\EventDispatcherInterface + */ + protected $eventDispatcher; + + /** * Constructs a FileUploadResource instance. * * @param array $configuration @@ -153,8 +163,10 @@ class FileUploadResource extends ResourceBase { * The lock service. * @param \Drupal\Core\Config\Config $system_file_config * The system file configuration. + * @param \Symfony\Component\EventDispatcher\EventDispatcherInterface $event_dispatcher + * The event dispatcher service. */ - public function __construct(array $configuration, $plugin_id, $plugin_definition, $serializer_formats, LoggerInterface $logger, FileSystemInterface $file_system, EntityTypeManagerInterface $entity_type_manager, EntityFieldManagerInterface $entity_field_manager, AccountInterface $current_user, MimeTypeGuesserInterface $mime_type_guesser, Token $token, LockBackendInterface $lock, Config $system_file_config) { + public function __construct(array $configuration, $plugin_id, $plugin_definition, $serializer_formats, LoggerInterface $logger, FileSystemInterface $file_system, EntityTypeManagerInterface $entity_type_manager, EntityFieldManagerInterface $entity_field_manager, AccountInterface $current_user, MimeTypeGuesserInterface $mime_type_guesser, Token $token, LockBackendInterface $lock, Config $system_file_config, EventDispatcherInterface $event_dispatcher = NULL) { parent::__construct($configuration, $plugin_id, $plugin_definition, $serializer_formats, $logger); $this->fileSystem = $file_system; $this->entityTypeManager = $entity_type_manager; @@ -164,6 +176,11 @@ public function __construct(array $configuration, $plugin_id, $plugin_definition $this->token = $token; $this->lock = $lock; $this->systemFileConfig = $system_file_config; + if (!$event_dispatcher) { + @trigger_error('The event_dispatcher service must be passed to FileUploadResource::__construct(), it is required before Drupal 9.0.0. See https://www.drupal.org/node/2972665.', E_USER_DEPRECATED); + $event_dispatcher = \Drupal::service('event_dispatcher'); + } + $this->eventDispatcher = $event_dispatcher; } /** @@ -183,7 +200,8 @@ public static function create(ContainerInterface $container, array $configuratio $container->get('file.mime_type.guesser'), $container->get('token'), $container->get('lock'), - $container->get('config.factory')->get('system.file') + $container->get('config.factory')->get('system.file'), + $container->get('event_dispatcher') ); } @@ -458,29 +476,14 @@ protected function validate(FileInterface $file, array $validators) { * The prepared/munged filename. */ protected function prepareFilename($filename, array &$validators) { - if (!empty($validators['file_validate_extensions'][0])) { - // If there is a file_validate_extensions validator and a list of - // valid extensions, munge the filename to protect against possible - // malicious extension hiding within an unknown file type. For example, - // "filename.html.foo". - $filename = file_munge_filename($filename, $validators['file_validate_extensions'][0]); + $extensions = empty($validators['file_validate_extensions'][0]) ? '' : $validators['file_validate_extensions'][0]; + $event = new FileUploadSanitizeNameEvent($filename, $extensions); + $this->eventDispatcher->dispatch(FileEvents::SANITIZE, $event); + $filename = $event->getFilenameWithExtension(); + if (isset($validators['file_validate_extensions'][0])) { + // @see \Drupal\Core\File\Event\FileUploadSanitizeNameEvent::makeTextFile() + $validators['file_validate_extensions'][0] = $event->getAllowedExtensions(); } - - // Rename potentially executable files, to help prevent exploits (i.e. will - // rename filename.php.foo and filename.php to filename.php.foo.txt and - // filename.php.txt, respectively). Don't rename if 'allow_insecure_uploads' - // evaluates to TRUE. - if (!$this->systemFileConfig->get('allow_insecure_uploads') && preg_match(FILE_INSECURE_EXTENSION_REGEX, $filename) && (substr($filename, -4) != '.txt')) { - // The destination filename will also later be used to create the URI. - $filename .= '.txt'; - - // The .txt extension may not be in the allowed list of extensions. We - // have to add it here or else the file upload will fail. - if (!empty($validators['file_validate_extensions'][0])) { - $validators['file_validate_extensions'][0] .= ' txt'; - } - } - return $filename; } diff --git a/core/modules/file/src/Tests/FileFieldWidgetTest.php b/core/modules/file/src/Tests/FileFieldWidgetTest.php index bef39f1..0c18eaf 100644 --- a/core/modules/file/src/Tests/FileFieldWidgetTest.php +++ b/core/modules/file/src/Tests/FileFieldWidgetTest.php @@ -259,7 +259,7 @@ public function testMultiValuedWidget() { '%field' => $field_name, '@max' => $cardinality, '@count' => count($upload_files_node_creation) + count($upload_files_node_revision), - '%list' => implode(', ', array_fill(0, 3, $test_file->getFilename())), + '%list' => implode(', ', ['text-0_2.txt', 'text-0_3.txt', 'text-0_4.txt']), ]; $this->assertRaw(t('Field %field can only hold @max values but there were @count uploaded. The following files have been omitted as a result: %list.', $args)); $node_storage->resetCache([$nid]); @@ -291,7 +291,7 @@ public function testMultiValuedWidget() { '%field' => $field_name, '@max' => $cardinality, '@count' => count($upload_files), - '%list' => $test_file->getFileName(), + '%list' => 'text-0_12.txt', ]; $this->assertRaw(t('Field %field can only hold @max values but there were @count uploaded. The following files have been omitted as a result: %list.', $args)); } diff --git a/core/modules/file/tests/src/Functional/SaveUploadTest.php b/core/modules/file/tests/src/Functional/SaveUploadTest.php index 552cfc6..f15323f 100644 --- a/core/modules/file/tests/src/Functional/SaveUploadTest.php +++ b/core/modules/file/tests/src/Functional/SaveUploadTest.php @@ -206,6 +206,7 @@ public function testHandleDangerousFile() { $this->assertResponse(200, 'Received a 200 response for posted test file.'); $message = t('For security reasons, your upload has been renamed to') . ' ' . $this->phpfile->filename . '.txt' . ''; $this->assertRaw($message, 'Dangerous file was renamed.'); + $this->assertSession()->pageTextContains('File name is php-2.php.txt.'); $this->assertRaw(t('File MIME type is text/plain.'), "Dangerous file's MIME type was changed."); $this->assertRaw(t('You WIN!'), 'Found the success message.'); @@ -221,7 +222,7 @@ public function testHandleDangerousFile() { $this->drupalPostForm('file-test/upload', $edit, t('Submit')); $this->assertResponse(200, 'Received a 200 response for posted test file.'); $this->assertNoRaw(t('For security reasons, your upload has been renamed'), 'Found no security message.'); - $this->assertRaw(t('File name is @filename', ['@filename' => $this->phpfile->filename]), 'Dangerous file was not renamed when insecure uploads is TRUE.'); + $this->assertSession()->pageTextContains('File name is php-2.php.'); $this->assertRaw(t('You WIN!'), 'Found the success message.'); // Check that the correct hooks were called. @@ -291,6 +292,7 @@ public function testExistingRename() { $this->drupalPostForm('file-test/upload', $edit, t('Submit')); $this->assertResponse(200, 'Received a 200 response for posted test file.'); $this->assertRaw(t('You WIN!'), 'Found the success message.'); + $this->assertSession()->pageTextContains('File name is image-test_0.png.'); // Check that the correct hooks were called. $this->assertFileHooksCalled(['validate', 'insert']); @@ -307,6 +309,7 @@ public function testExistingReplace() { $this->drupalPostForm('file-test/upload', $edit, t('Submit')); $this->assertResponse(200, 'Received a 200 response for posted test file.'); $this->assertRaw(t('You WIN!'), 'Found the success message.'); + $this->assertSession()->pageTextContains('File name is image-test.png.'); // Check that the correct hooks were called. $this->assertFileHooksCalled(['validate', 'load', 'update']); diff --git a/core/modules/layout_builder/tests/src/FunctionalJavascript/InlineBlockPrivateFilesTest.php b/core/modules/layout_builder/tests/src/FunctionalJavascript/InlineBlockPrivateFilesTest.php index a94be8b..6733194 100644 --- a/core/modules/layout_builder/tests/src/FunctionalJavascript/InlineBlockPrivateFilesTest.php +++ b/core/modules/layout_builder/tests/src/FunctionalJavascript/InlineBlockPrivateFilesTest.php @@ -139,7 +139,7 @@ public function testPrivateFiles() { $assert_session->pageTextContains('You are not authorized to access this page'); $this->drupalGet('node/2/layout'); - $file4 = $this->createPrivateFile('drupal.txt'); + $file4 = $this->createPrivateFile('drupal_4.txt'); $this->addInlineFileBlockToLayout('The file', $file4); $this->assertSaveLayout(); diff --git a/core/modules/system/src/EventSubscriber/FileEventSubscriber.php b/core/modules/system/src/EventSubscriber/FileEventSubscriber.php new file mode 100644 index 0000000..c98f03b --- /dev/null +++ b/core/modules/system/src/EventSubscriber/FileEventSubscriber.php @@ -0,0 +1,106 @@ +config = $config_factory->get('system.file'); + } + + /** + * {@inheritdoc} + */ + public static function getSubscribedEvents() { + $events[FileEvents::SANITIZE][] = ['security', 1024]; + return $events; + } + + /** + * Sanitize the filename of a file being uploaded. + * + * @param \Drupal\Core\File\Event\FileUploadSanitizeNameEvent $event + * File upload sanitize name event. + */ + public function security(FileUploadSanitizeNameEvent $event) { + // Don't rename if 'allow_insecure_uploads' evaluates to TRUE. + if ($this->config->get('allow_insecure_uploads')) { + $event->lockExtension(); + return; + } + + // Remove any null bytes. See + // http://php.net/manual/security.filesystem.nullbytes.php + $filename = str_replace(chr(0), '', $event->getFilename()); + $extension = str_replace(chr(0), '', $event->getExtension()); + + // Munge the filename to protect against possible malicious extension hiding + // within an unknown file type (ie: filename.html.foo). + $extensions = $event->getAllowedExtensions(); + // Allow potentially insecure uploads for very savvy users and admin + if (!empty($extensions)) { + $whitelist = array_unique(explode(' ', strtolower(trim($extensions)))); + + // Split the filename up by periods. The first part becomes the basename + // the last part the final extension. + $filename_parts = explode('.', $filename); + // Remove file basename. + $filename = array_shift($filename_parts); + + // Loop through the middle parts of the name and add an underscore to the + // end of each section that could be a file extension but isn't in the + // list of allowed extensions. + foreach ($filename_parts as $filename_part) { + $filename .= '.' . $filename_part; + if (!in_array(strtolower($filename_part), $whitelist) && preg_match("/^[a-zA-Z]{2,5}\d?$/", $filename_part)) { + $filename .= '_'; + } + } + } + if ($filename . '.' . $extension !== $event->getFilenameWithExtension()) { + $event + ->setFilename($filename) + ->setExtension($extension) + ->setSecurityRename(); + } + + // Rename potentially executable files, to help prevent exploits (i.e. will + // rename filename.php.foo and filename.php to filename.php.foo.txt and + // filename.php.txt, respectively). + if (preg_match(static::INSECURE_EXTENSION_REGEX, $event->getFilenameWithExtension()) && $event->getExtension() !== 'txt') { + $event + ->makeTextFile() + ->setSecurityRename(); + } + $event->lockExtension(); + } + +} diff --git a/core/modules/system/system.services.yml b/core/modules/system/system.services.yml index ccfabe8..6caff10 100644 --- a/core/modules/system/system.services.yml +++ b/core/modules/system/system.services.yml @@ -43,3 +43,8 @@ services: arguments: ['@theme_handler', '@cache_tags.invalidator'] tags: - { name: event_subscriber } + system.file_event.subscriber: + class: Drupal\system\EventSubscriber\FileEventSubscriber + arguments: ['@config.factory'] + tags: + - { name: event_subscriber } diff --git a/core/modules/system/tests/src/Unit/Event/FileEventSubscriberTest.php b/core/modules/system/tests/src/Unit/Event/FileEventSubscriberTest.php new file mode 100644 index 0000000..bd4fa34 --- /dev/null +++ b/core/modules/system/tests/src/Unit/Event/FileEventSubscriberTest.php @@ -0,0 +1,80 @@ +getConfigFactoryStub([ + 'system.file' => [ + 'allow_insecure_uploads' => $allow_insecure_uploads, + ], + ]); + + $event = new FileUploadSanitizeNameEvent($original, $extensions); + $subscriber = new FileEventSubscriber($config_factory); + $subscriber->security($event); + + // Check the results of the configured sanitization. + $this->assertSame($expected, $event->getFilenameWithExtension()); + + $this->assertSame($expected_security_rename, $event->isSecurityRename()); + + // Ensure that regardless of whether the filename has changed that after + // running FileEventSubscriber::security() on the event that the extension + // is locked. + $this->setExpectedException(ExtensionLockedException::class); + $event->setExtension('php'); + } + + /** + * Provides data for testSecurity(). + * + * @return array + * Arrays with original name, allowed extensions, expected name, whether + * insecure uploads are allowed and the expected security rename flag. + */ + public function provideFilenames() { + return [ + ['foo.txt', '', 'foo.txt', FALSE, FALSE], + ['foo.txt', '', 'foo.txt', TRUE, FALSE], + ['foo.php', '', 'foo.php.txt', FALSE, TRUE], + ['foo.php', '', 'foo.php', TRUE, FALSE], + ['foo.phar.png.php.jpg', 'jpg png', 'foo.phar_.png.php_.jpg', FALSE, TRUE], + ['foo.phar.png.php.jpg', 'jpg png', 'foo.phar.png.php.jpg', TRUE, FALSE], + // @todo this is an existing logic bug that is harder to reveal. + ['foo.php.php', 'jpg', 'foo.php_.php.txt', FALSE, TRUE], + ['foo' . chr(0) . '.txt' . chr(0), '', 'foo.txt', FALSE, TRUE], + ['foo' . chr(0) . '.txt' . chr(0), '', 'foo' . chr(0) . '.txt' . chr(0), TRUE, FALSE], + ]; + } + +} diff --git a/core/tests/Drupal/KernelTests/Core/File/NameMungingTest.php b/core/tests/Drupal/KernelTests/Core/File/NameMungingTest.php index 3cf351b..90e66ea 100644 --- a/core/tests/Drupal/KernelTests/Core/File/NameMungingTest.php +++ b/core/tests/Drupal/KernelTests/Core/File/NameMungingTest.php @@ -6,6 +6,7 @@ * Tests filename munging and unmunging. * * @group File + * @group legacy */ class NameMungingTest extends FileTestBase { @@ -33,6 +34,8 @@ protected function setUp() { /** * Create a file and munge/unmunge the name. + * + * @expectedDeprecation file_munge_filename() is deprecated in Drupal 8.7.0 and will be removed before Drupal 9.0.0. Dispatch a \Drupal\Core\File\Event\FileEvents::SANITIZE event instead. See https://www.drupal.org/node/3032541 */ public function testMunging() { // Disable insecure uploads. @@ -78,6 +81,8 @@ public function testMungeIgnoreWhitelisted() { /** * Ensure that unmunge gets your name back. + * + * @expectedDeprecation file_unmunge_filename() is deprecated in Drupal 8.7.0 and will be removed before Drupal 9.0.0. Use str_replace() instead. See https://www.drupal.org/node/3032541 */ public function testUnMunge() { $munged_name = file_munge_filename($this->name, '', FALSE); diff --git a/core/tests/Drupal/Tests/Core/File/FileUploadSanitizeNameEventTest.php b/core/tests/Drupal/Tests/Core/File/FileUploadSanitizeNameEventTest.php new file mode 100644 index 0000000..d47126f --- /dev/null +++ b/core/tests/Drupal/Tests/Core/File/FileUploadSanitizeNameEventTest.php @@ -0,0 +1,108 @@ +assertSame('bar', $event->getExtension()); + $event->setExtension('txt'); + $this->assertSame('foo.txt', $event->getFilenameWithExtension()); + $this->assertSame('txt', $event->getExtension()); + $event->lockExtension(); + $this->setExpectedException(ExtensionLockedException::class); + $event->setExtension('bar'); + } + + /** + * Tests making a text file. + * + * @covers ::makeTextFile + */ + public function testMakeTextFile() { + $event = new FileUploadSanitizeNameEvent('foo.bar', ''); + $event->makeTextFile(); + $this->assertSame('foo.bar.txt', $event->getFilenameWithExtension()); + $this->assertSame('foo.bar', $event->getFilename()); + $this->assertSame('txt', $event->getExtension()); + $this->assertSame('', $event->getAllowedExtensions()); + + $event = new FileUploadSanitizeNameEvent('foo.bar', 'jpg'); + $event->makeTextFile(); + $this->assertSame('foo.bar.txt', $event->getFilenameWithExtension()); + $this->assertSame('foo.bar', $event->getFilename()); + $this->assertSame('txt', $event->getExtension()); + $this->assertSame('jpg txt', $event->getAllowedExtensions()); + } + + /** + * Test event construction. + * + * @dataProvider provideFilenames + * @covers ::__construct + * @covers ::getFilename + * @covers ::getExtension + * @covers ::getFilenameWithExtension + */ + public function testEventFilenameFunctions($filename, $expected_filename, $expected_extension) { + $event = new FileUploadSanitizeNameEvent($filename, ''); + $this->assertSame($expected_filename, $event->getFilename()); + $this->assertSame($expected_extension, $event->getExtension()); + $this->assertSame($filename, $event->getFilenameWithExtension()); + } + + /** + * Provides data for testEvent(). + * + * @return array + * Arrays with original name, expected name and allowed extensions (as a + * space-delimited string). + */ + public function provideFilenames() { + return [ + 'ASCII filename with extension' => [ + 'example.txt', + 'example', + 'txt', + ], + 'ASCII filename with complex extension' => [ + 'example.html.twig', + 'example.html', + 'twig', + ], + 'ASCII filename with lots of dots' => [ + 'dotty.....txt', + 'dotty....', + 'txt', + ], + 'Unicode filename with extension' => [ + 'Ä Ö Ü Å Ø äöüåøhello.txt', + 'Ä Ö Ü Å Ø äöüåøhello', + 'txt', + ], + 'Unicode filename without extension' => [ + 'Ä Ö Ü Å Ø äöüåøhello', + 'Ä Ö Ü Å Ø äöüåøhello', + '', + ], + ]; + } + +}