diff --git a/src/Hook/FileFieldPathsProcessFileLegacy.php b/src/Hook/FileFieldPathsProcessFileLegacy.php index 804da51..c6229d4 100644 --- a/src/Hook/FileFieldPathsProcessFileLegacy.php +++ b/src/Hook/FileFieldPathsProcessFileLegacy.php @@ -4,6 +4,7 @@ namespace Drupal\filefield_paths\Hook; +use Drupal\Core\Config\ConfigFactoryInterface; use Drupal\Component\Utility\DeprecationHelper; use Drupal\Core\Entity\ContentEntityInterface; use Drupal\Core\Extension\ModuleHandlerInterface; @@ -28,6 +29,7 @@ final readonly class FileFieldPathsProcessFileLegacy { public function __construct( + private ConfigFactoryInterface $configFactory, private FileSystemInterface $fileSystem, private FileRepositoryInterface $fileRepository, private StreamWrapperManagerInterface $streamWrapperManager, @@ -155,10 +157,13 @@ public function fileFieldPathsProcessFile(ContentEntityInterface $entity, FileFi } $this->processOutcome->recordUpdated($file->id()); - // Create redirect from old location. + // Create redirect from old location, unless the file is only now + // leaving the staging area. A staged path exists between the upload + // and the save, so nothing can be linking to it. if ( !empty($settings['redirect']) && $settings['active_updating'] && - $this->moduleHandler->moduleExists('redirect') + $this->moduleHandler->moduleExists('redirect') && + !$this->isStagedUpload($file->getFileUri(), $settings) ) { $redirect = $this->getRedirect(); $redirect->createRedirect($file->getFileUri(), $new_file->getFileUri(), $file->language()); @@ -187,4 +192,34 @@ private function getRedirect(): RedirectInterface { return ($this->redirectClosure)(); } + /** + * Checks whether a file is still at the upload staging location. + * + * @param string $uri + * The file URI before the move. + * @param array $settings + * The File (Field) Paths settings for the field. + * + * @return bool + * TRUE if the file has not left the staging location yet. + */ + private function isStagedUpload(string $uri, array $settings): bool { + $temp_location = $settings['temp_location'] ?? NULL; + if (empty($temp_location)) { + $temp_location = $this->configFactory + ->get('filefield_paths.settings') + ->get('temp_location'); + } + if (!is_string($temp_location) || $temp_location === '') { + return FALSE; + } + // A bare scheme root such as "public://" is not a staging directory, and + // the settings form accepts one. Used as a prefix it would match every + // file on that scheme and stop redirects being created at all. + if ((string) $this->streamWrapperManager::getTarget($temp_location) === '') { + return FALSE; + } + return str_starts_with($uri, rtrim($temp_location, '/') . '/'); + } + } diff --git a/tests/src/Kernel/FileFieldPathsProcessFileLegacyTest.php b/tests/src/Kernel/FileFieldPathsProcessFileLegacyTest.php index 7443371..b042024 100644 --- a/tests/src/Kernel/FileFieldPathsProcessFileLegacyTest.php +++ b/tests/src/Kernel/FileFieldPathsProcessFileLegacyTest.php @@ -158,6 +158,7 @@ protected function getService(): FileFieldPathsProcessFileLegacy { */ protected function constructService(?FileSystemInterface $fileSystem = NULL, ?FileRepositoryInterface $fileRepository = NULL): FileFieldPathsProcessFileLegacy { return new FileFieldPathsProcessFileLegacy( + $this->container->get('config.factory'), $fileSystem ?? $this->container->get('file_system'), $fileRepository ?? $this->container->get('file.repository'), $this->container->get('stream_wrapper_manager'), diff --git a/tests/src/Kernel/StagingRedirectTest.php b/tests/src/Kernel/StagingRedirectTest.php new file mode 100644 index 0000000..0c2f59f --- /dev/null +++ b/tests/src/Kernel/StagingRedirectTest.php @@ -0,0 +1,184 @@ + + */ + protected static $modules = [ + 'system', + 'user', + 'field', + 'file', + 'path_alias', + 'redirect', + 'link', + 'entity_test', + 'filefield_paths', + ]; + + /** + * {@inheritdoc} + */ + protected function setUp(): void { + parent::setUp(); + $this->installEntitySchema('user'); + $this->installEntitySchema('file'); + $this->installEntitySchema('entity_test'); + $this->installEntitySchema('redirect'); + $this->installEntitySchema('path_alias'); + $this->installSchema('file', ['file_usage']); + $this->installConfig(['filefield_paths']); + $this->config('redirect.settings')->set('default_status_code', 301)->save(); + + FieldStorageConfig::create([ + 'field_name' => 'field_file', + 'entity_type' => 'entity_test', + 'type' => 'file', + 'cardinality' => FieldStorageDefinitionInterface::CARDINALITY_UNLIMITED, + 'settings' => ['uri_scheme' => 'public'], + ])->save(); + + $options = ['slashes' => FALSE, 'pathauto' => FALSE, 'transliterate' => FALSE]; + $field = FieldConfig::create([ + 'entity_type' => 'entity_test', + 'field_name' => 'field_file', + 'bundle' => 'entity_test', + ]); + $field->setThirdPartySetting('filefield_paths', 'enabled', TRUE); + $field->setThirdPartySetting('filefield_paths', 'file_path', [ + 'value' => 'sorted', + 'options' => $options, + ]); + $field->setThirdPartySetting('filefield_paths', 'file_name', [ + 'value' => '', + 'options' => $options, + ]); + $field->setThirdPartySetting('filefield_paths', 'active_updating', TRUE); + $field->setThirdPartySetting('filefield_paths', 'redirect', TRUE); + $field->setThirdPartySetting('filefield_paths', 'retroactive_update', FALSE); + $field->save(); + } + + /** + * Attaches a file at the given URI to a new entity and saves it. + * + * @param string $uri + * Where the file sits before the entity is saved. + */ + private function attachFileAt(string $uri): void { + $file_system = $this->container->get('file_system'); + $directory = $file_system->dirname($uri); + $file_system->prepareDirectory($directory, $file_system::CREATE_DIRECTORY); + file_put_contents($uri, 'contents'); + $file = File::create(['uri' => $uri]); + $file->setPermanent(); + $file->save(); + + EntityTest::create([ + 'name' => 'test', + 'field_file' => [['target_id' => $file->id()]], + ])->save(); + } + + /** + * Counts the redirects that exist. + */ + private function redirectCount(): int { + return count($this->container->get('entity_type.manager') + ->getStorage('redirect') + ->loadMultiple()); + } + + /** + * A file leaving the staging area earns no redirect. + * + * The staging path only ever existed between the upload and the save, so + * nothing can be linking to it. + * + * @see https://www.drupal.org/i/3494240 + */ + public function testNoRedirectWhenTheFileComesFromStaging(): void { + $this->attachFileAt('public://filefield_paths/example.txt'); + + $this->assertFileExists('public://sorted/example.txt'); + $this->assertSame(0, $this->redirectCount(), 'A staged upload should not leave a redirect behind.'); + } + + /** + * A file moved from a real location still earns a redirect. + * + * This is the control. The fix must not stop redirects for files that were + * genuinely reachable at their old path. + * + * @see https://www.drupal.org/i/3494240 + */ + public function testRedirectWhenTheFileWasAlreadyPublished(): void { + $this->attachFileAt('public://published/example.txt'); + + $this->assertFileExists('public://sorted/example.txt'); + $this->assertSame(1, $this->redirectCount(), 'A move from a real location should still leave a redirect.'); + } + + /** + * A bare scheme root is not a staging location. + * + * The settings form accepts "public://" on its own. Treating that as a + * staging prefix would match every file on the scheme and quietly stop + * redirects being created at all. + * + * @see https://www.drupal.org/i/3494240 + */ + public function testBareSchemeRootIsNotTreatedAsStaging(): void { + $this->config('filefield_paths.settings') + ->set('temp_location', 'public://') + ->save(); + + $this->attachFileAt('public://published/example.txt'); + + $this->assertFileExists('public://sorted/example.txt'); + $this->assertSame(1, $this->redirectCount(), 'A bare scheme root must not suppress redirects.'); + } + + /** + * The field's own staging location wins over the global one. + * + * @see https://www.drupal.org/i/3494240 + */ + public function testFieldStagingLocationTakesPrecedence(): void { + $field = FieldConfig::loadByName('entity_test', 'entity_test', 'field_file'); + \assert($field instanceof FieldConfig); + $field->setThirdPartySetting('filefield_paths', 'temp_location', 'public://custom_stage'); + $field->save(); + $this->container->get('entity_field.manager')->clearCachedFieldDefinitions(); + + $this->attachFileAt('public://custom_stage/example.txt'); + + $this->assertFileExists('public://sorted/example.txt'); + $this->assertSame(0, $this->redirectCount(), 'The field level staging location should suppress the redirect.'); + } + +}