Issue #3389016 by kim.pepper, xjm, larowlan: Add file upload lock handling to FileUploadHandler

core/lib/Drupal/Core/Lock/LockAcquiringException.php

@@ -0,0 +1,11 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Drupal\Core\Lock;
+
+/**
+ * LockAcquiringException is thrown when a lock cannot be acquired.
+ */
+class LockAcquiringException extends \RuntimeException {
+}

core/modules/file/file.module

@@ -17,6 +17,7 @@ use Drupal\Core\File\Exception\InvalidStreamWrapperException;
 use Drupal\Core\File\FileSystemInterface;
 use Drupal\Core\Form\FormStateInterface;
 use Drupal\Core\Link;
+use Drupal\Core\Lock\LockAcquiringException;
 use Drupal\Core\Messenger\MessengerInterface;
 use Drupal\Core\Render\BubbleableMetadata;
 use Drupal\Core\Render\Element;
@@ -712,6 +713,10 @@ function file_save_upload($form_field_name, $validators = [], $destination = FAL
       \Drupal::messenger()->addError(t('The file %filename could not be uploaded because the name is invalid.', ['%filename' => $uploaded_file->getClientOriginalName()]));
       $files[$i] = FALSE;
     }
+    catch (LockAcquiringException $e) {
+      \Drupal::messenger()->addError(t('File already locked for writing.'));
+      $files[$i] = FALSE;
+    }
   }
 
   // Add files to the cache.

core/modules/file/file.services.yml

@@ -11,7 +11,7 @@ services:
       - { name: backend_overridable }
   file.upload_handler:
     class: Drupal\file\Upload\FileUploadHandler
-    arguments: ['@file_system', '@entity_type.manager', '@stream_wrapper_manager', '@event_dispatcher', '@file.mime_type.guesser', '@current_user', '@request_stack', '@file.repository', '@file.validator']
+    arguments: ['@file_system', '@entity_type.manager', '@stream_wrapper_manager', '@event_dispatcher', '@file.mime_type.guesser', '@current_user', '@request_stack', '@file.repository', '@file.validator', '@lock']
   Drupal\file\Upload\FileUploadHandler: '@file.upload_handler'
   file.repository:
     class: Drupal\file\FileRepository

core/modules/file/src/Upload/FileUploadHandler.php

@@ -2,12 +2,15 @@
 
 namespace Drupal\file\Upload;
 
+use Drupal\Component\Utility\Crypt;
 use Drupal\Core\Entity\EntityTypeManagerInterface;
 use Drupal\Core\File\Event\FileUploadSanitizeNameEvent;
 use Drupal\Core\File\Exception\FileExistsException;
 use Drupal\Core\File\Exception\FileWriteException;
 use Drupal\Core\File\Exception\InvalidStreamWrapperException;
 use Drupal\Core\File\FileSystemInterface;
+use Drupal\Core\Lock\LockAcquiringException;
+use Drupal\Core\Lock\LockBackendInterface;
 use Drupal\Core\Session\AccountInterface;
 use Drupal\Core\StreamWrapper\StreamWrapperManagerInterface;
 use Drupal\file\Entity\File;
@@ -120,8 +123,21 @@ class FileUploadHandler {
    *   The file repository.
    * @param \Drupal\file\Validation\FileValidatorInterface|null $file_validator
    *   The file validator.
+   * @param \Drupal\Core\Lock\LockBackendInterface|null $lock
+   *   The lock.
    */
-  public function __construct(FileSystemInterface $fileSystem, EntityTypeManagerInterface $entityTypeManager, StreamWrapperManagerInterface $streamWrapperManager, EventDispatcherInterface $eventDispatcher, MimeTypeGuesserInterface $mimeTypeGuesser, AccountInterface $currentUser, RequestStack $requestStack, FileRepositoryInterface $fileRepository = NULL, FileValidatorInterface $file_validator = NULL) {
+  public function __construct(
+    FileSystemInterface $fileSystem,
+    EntityTypeManagerInterface $entityTypeManager,
+    StreamWrapperManagerInterface $streamWrapperManager,
+    EventDispatcherInterface $eventDispatcher,
+    MimeTypeGuesserInterface $mimeTypeGuesser,
+    AccountInterface $currentUser,
+    RequestStack $requestStack,
+    FileRepositoryInterface $fileRepository = NULL,
+    FileValidatorInterface $file_validator = NULL,
+    protected ?LockBackendInterface $lock = NULL,
+  ) {
     $this->fileSystem = $fileSystem;
     $this->entityTypeManager = $entityTypeManager;
     $this->streamWrapperManager = $streamWrapperManager;
@@ -139,6 +155,10 @@ class FileUploadHandler {
       $file_validator = \Drupal::service('file.validator');
     }
     $this->fileValidator = $file_validator;
+    if (!$this->lock) {
+      @trigger_error('Calling ' . __METHOD__ . '() without the $lock argument is deprecated in drupal:10.3.0 and is required in drupal:11.0.0. See https://www.drupal.org/node/3389017', E_USER_DEPRECATED);
+      $this->lock = \Drupal::service('lock');
+    }
   }
 
   /**
@@ -170,6 +190,8 @@ class FileUploadHandler {
    *    Thrown when a file system error occurs and $throws is TRUE.
    * @throws \Drupal\file\Upload\FileValidationException
    *    Thrown when file validation fails and $throws is TRUE.
+   * @throws \Drupal\Core\Lock\LockAcquiringException
+   *   Thrown when a lock cannot be acquired.
    */
   public function handleFileUpload(UploadedFileInterface $uploadedFile, array $validators = [], string $destination = 'temporary://', int $replace = FileSystemInterface::EXISTS_REPLACE, bool $throw = TRUE): FileUploadResult {
     $originalName = $uploadedFile->getClientOriginalName();
@@ -236,6 +258,19 @@ class FileUploadHandler {
       throw new FileExistsException(sprintf('Destination file "%s" exists', $destinationFilename));
     }
 
+    // Lock based on the prepared file URI.
+    $lock_id = $this->generateLockId($destinationFilename);
+
+    try {
+      if (!$this->lock->acquire($lock_id)) {
+        throw new LockAcquiringException(
+          sprintf(
+            'File "%s" is already locked for writing.',
+            $destinationFilename
+          )
+        );
+      }
+
       $file = File::create([
         'uid' => $this->currentUser->id(),
         'status' => 0,
@@ -256,6 +291,7 @@ class FileUploadHandler {
       $violations = $this->fileValidator->validate($file, $validators);
       if (count($violations) > 0) {
         $result->addViolations($violations);
+
         return $result;
       }
 
@@ -265,18 +301,24 @@ class FileUploadHandler {
           $errors[] = $violation->getMessage();
         }
         if (!empty($errors)) {
-        throw new FileValidationException('File validation failed', $filename, $errors);
+          throw new FileValidationException(
+            'File validation failed',
+            $filename,
+            $errors
+          );
         }
       }
 
       $file->setFileUri($destinationFilename);
 
       if (!$this->moveUploadedFile($uploadedFile, $file->getFileUri())) {
-      throw new FileWriteException('File upload error. Could not move uploaded file.');
+        throw new FileWriteException(
+          'File upload error. Could not move uploaded file.'
+        );
       }
 
-    // Update the filename with any changes as a result of security or renaming
+      // Update the filename with any changes as a result of security or
-    // due to an existing file.
+      // renaming due to an existing file.
       $file->setFilename($this->fileSystem->basename($file->getFileUri()));
 
       if ($replace === FileSystemInterface::EXISTS_REPLACE) {
@@ -306,11 +348,16 @@ class FileUploadHandler {
           $errors[] = $violation->getMessage();
         }
         if (!empty($errors)) {
-        throw new FileValidationException('File validation failed', $filename, $errors);
+          throw new FileValidationException(
+            'File validation failed',
+            $filename,
+            $errors
+          );
         }
       }
       if (count($violations) > 0) {
         $result->addViolations($violations);
+
         return $result;
       }
 
@@ -325,7 +372,10 @@ class FileUploadHandler {
         $allowed_temp_files[$file->id()] = $file->id();
         $session->set('anonymous_allowed_file_ids', $allowed_temp_files);
       }
-
+    }
+    finally {
+      $this->lock->release($lock_id);
+    }
     return $result;
   }
 
@@ -412,4 +462,11 @@ class FileUploadHandler {
     return $this->fileRepository->loadByUri($uri);
   }
 
+  /**
+   * Generates a lock ID based on the file URI.
+   */
+  protected static function generateLockId(string $fileUri): string {
+    return 'file:upload:' . Crypt::hashBase64($fileUri);
+  }
+
 }

core/modules/file/tests/src/Kernel/FileUploadHandlerTest.php

@@ -2,6 +2,8 @@
 
 namespace Drupal\Tests\file\Kernel;
 
+use Drupal\Core\Lock\LockAcquiringException;
+use Drupal\Core\Lock\LockBackendInterface;
 use Drupal\file\Upload\FileUploadHandler;
 use Drupal\file\Upload\UploadedFileInterface;
 use Drupal\KernelTests\KernelTestBase;
@@ -70,4 +72,35 @@ class FileUploadHandlerTest extends KernelTestBase {
     $this->assertEquals(['txt'], $subscriber->getAllowedExtensions());
   }
 
+  /**
+   * Test the lock acquire exception.
+   */
+  public function testLockAcquireException(): void {
+
+    $lock = $this->createMock(LockBackendInterface::class);
+    $lock->expects($this->once())->method('acquire')->willReturn(FALSE);
+
+    $fileUploadHandler = new FileUploadHandler(
+      $this->container->get('file_system'),
+      $this->container->get('entity_type.manager'),
+      $this->container->get('stream_wrapper_manager'),
+      $this->container->get('event_dispatcher'),
+      $this->container->get('file.mime_type.guesser'),
+      $this->container->get('current_user'),
+      $this->container->get('request_stack'),
+      $this->container->get('file.repository'),
+      $this->container->get('file.validator'),
+      $lock
+    );
+
+    $file_name = $this->randomMachineName();
+    $file_info = $this->createMock(UploadedFileInterface::class);
+    $file_info->expects($this->once())->method('getClientOriginalName')->willReturn($file_name);
+
+    $this->expectException(LockAcquiringException::class);
+    $this->expectExceptionMessage(sprintf('File "temporary://%s" is already locked for writing.', $file_name));
+
+    $fileUploadHandler->handleFileUpload(uploadedFile: $file_info, throw: FALSE);
+  }
+
 }