fix(#3494240): skip the redirect when a file leaves the staging area

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, '/') . '/');
+  }
+
 }

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'),

tests/src/Kernel/StagingRedirectTest.php

@@ -0,0 +1,184 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Drupal\Tests\filefield_paths\Kernel;
+
+use Drupal\Core\Field\FieldStorageDefinitionInterface;
+use Drupal\KernelTests\KernelTestBase;
+use Drupal\entity_test\Entity\EntityTest;
+use Drupal\field\Entity\FieldConfig;
+use Drupal\field\Entity\FieldStorageConfig;
+use Drupal\file\Entity\File;
+use PHPUnit\Framework\Attributes\Group;
+use PHPUnit\Framework\Attributes\RunTestsInSeparateProcesses;
+
+/**
+ * Tests which moves earn a redirect.
+ *
+ * @group filefield_paths
+ * @covers \Drupal\filefield_paths\Hook\FileFieldPathsProcessFileLegacy
+ */
+#[Group('filefield_paths')]
+#[RunTestsInSeparateProcesses]
+class StagingRedirectTest extends KernelTestBase {
+
+  /**
+   * Modules to enable.
+   *
+   * @var array<string>
+   */
+  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.');
+  }
+
+}