Issue #3380379 by kim.pepper, smustgrave, quietone: Create content disposition...

Issue #3380379 by kim.pepper, smustgrave, quietone: Create content disposition filename extractor and deprecate duplicate code in REST and JSON API

core/modules/file/src/Plugin/rest/resource/FileUploadResource.php

@@ -17,6 +17,7 @@
 use Drupal\Core\Session\AccountInterface;
 use Drupal\Core\Utility\Token;
 use Drupal\file\Entity\File;
+use Drupal\file\Upload\ContentDispositionFilenameParser;
 use Drupal\file\Upload\InputStreamFileWriterInterface;
 use Drupal\file\Validation\FileValidatorInterface;
 use Drupal\rest\ModifiedResourceResponse;
@@ -30,7 +31,6 @@
 use Symfony\Component\HttpFoundation\File\Exception\UploadException;
 use Symfony\Component\HttpFoundation\Request;
 use Symfony\Component\HttpKernel\Exception\AccessDeniedHttpException;
-use Symfony\Component\HttpKernel\Exception\BadRequestHttpException;
 use Symfony\Component\HttpKernel\Exception\HttpException;
 use Symfony\Component\HttpKernel\Exception\NotFoundHttpException;
 use Symfony\Component\HttpKernel\Exception\UnprocessableEntityHttpException;
@@ -67,6 +67,12 @@ class FileUploadResource extends ResourceBase {
    * The regex used to extract the filename from the content disposition header.
    *
    * @var string
+   *
+   * @deprecated in drupal:10.3.0 and is removed from drupal:11.0.0. Use
+   *   \Drupal\file\Upload\ContentDispositionFilenameParser::REQUEST_HEADER_FILENAME_REGEX
+   *   instead.
+   *
+   * @see https://www.drupal.org/node/3380380
    */
   const REQUEST_HEADER_FILENAME_REGEX = '@\bfilename(?<star>\*?)=\"(?<filename>.+)\"@';
 
@@ -265,7 +271,7 @@ public function permissions() {
    *   or when temporary files cannot be moved to their new location.
    */
   public function post(Request $request, $entity_type_id, $bundle, $field_name) {
-    $filename = $this->validateAndParseContentDispositionHeader($request);
+    $filename = ContentDispositionFilenameParser::parseFilename($request);
 
     $field_definition = $this->validateAndLoadFieldDefinition($entity_type_id, $bundle, $field_name);
 
@@ -389,35 +395,16 @@ protected function streamUploadData(): string {
    *
    * @throws \Symfony\Component\HttpKernel\Exception\BadRequestHttpException
    *   Thrown when the 'Content-Disposition' request header is invalid.
+   *
+   * @deprecated in drupal:10.3.0 and is removed from drupal:11.0.0. Use
+   *   \Drupal\file\Upload\ContentDispositionFilenameParser::parseFilename()
+   *   instead.
+   *
+   * @see https://www.drupal.org/node/3380380
    */
   protected function validateAndParseContentDispositionHeader(Request $request) {
-    // Firstly, check the header exists.
-    if (!$request->headers->has('content-disposition')) {
-      throw new BadRequestHttpException('"Content-Disposition" header is required. A file name in the format "filename=FILENAME" must be provided');
-    }
-
-    $content_disposition = $request->headers->get('content-disposition');
-
-    // Parse the header value. This regex does not allow an empty filename.
-    // i.e. 'filename=""'. This also matches on a word boundary so other keys
-    // like 'not_a_filename' don't work.
-    if (!preg_match(static::REQUEST_HEADER_FILENAME_REGEX, $content_disposition, $matches)) {
-      throw new BadRequestHttpException('No filename found in "Content-Disposition" header. A file name in the format "filename=FILENAME" must be provided');
-    }
-
-    // Check for the "filename*" format. This is currently unsupported.
-    if (!empty($matches['star'])) {
-      throw new BadRequestHttpException('The extended "filename*" format is currently not supported in the "Content-Disposition" header');
-    }
-
-    // Don't validate the actual filename here, that will be done by the upload
-    // validators in validate().
-    // @see \Drupal\file\Plugin\rest\resource\FileUploadResource::validate()
-    $filename = $matches['filename'];
-
-    // Make sure only the filename component is returned. Path information is
-    // stripped as per https://tools.ietf.org/html/rfc6266#section-4.3.
-    return $this->fileSystem->basename($filename);
+    @trigger_error('Calling ' . __METHOD__ . '() is deprecated in drupal:10.3.0 and is removed from drupal:11.0.0. Use \Drupal\file\Upload\ContentDispositionFilenameParser::parseFilename() instead. See https://www.drupal.org/node/3380380', E_USER_DEPRECATED);
+    return ContentDispositionFilenameParser::parseFilename($request);
   }
 
   /**

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

+<?php
+
+namespace Drupal\file\Upload;
+
+use Symfony\Component\HttpFoundation\Request;
+use Symfony\Component\HttpKernel\Exception\BadRequestHttpException;
+
+/**
+ * Parses the content-disposition header to extract the client filename.
+ */
+final class ContentDispositionFilenameParser {
+
+  /**
+   * The regex used to extract the filename from the content disposition header.
+   */
+  const REQUEST_HEADER_FILENAME_REGEX = '@\bfilename(?<star>\*?)=\"(?<filename>.+)\"@';
+
+  /**
+   * Private constructor to prevent instantiation.
+   */
+  private function __construct() {}
+
+  /**
+   * Parse the content disposition header and return the filename.
+   *
+   * @param \Symfony\Component\HttpFoundation\Request $request
+   *   The request.
+   *
+   * @return string
+   *   The filename.
+   *
+   * @throws \Symfony\Component\HttpKernel\Exception\BadRequestHttpException
+   *   Thrown when the 'Content-Disposition' request header is invalid.
+   */
+  public static function parseFilename(Request $request): string {
+    // Firstly, check the header exists.
+    if (!$request->headers->has('content-disposition')) {
+      throw new BadRequestHttpException('"Content-Disposition" header is required. A file name in the format "filename=FILENAME" must be provided.');
+    }
+
+    $content_disposition = $request->headers->get('content-disposition');
+
+    // Parse the header value. This regex does not allow an empty filename.
+    // i.e. 'filename=""'. This also matches on a word boundary so other keys
+    // like 'not_a_filename' don't work.
+    if (!preg_match(static::REQUEST_HEADER_FILENAME_REGEX, $content_disposition, $matches)) {
+      throw new BadRequestHttpException('No filename found in "Content-Disposition" header. A file name in the format "filename=FILENAME" must be provided.');
+    }
+
+    // Check for the "filename*" format. This is currently unsupported.
+    if (!empty($matches['star'])) {
+      throw new BadRequestHttpException('The extended "filename*" format is currently not supported in the "Content-Disposition" header.');
+    }
+
+    // Don't validate the actual filename here, that will be done by the upload
+    // validators in validate().
+    // @see \Drupal\file\Plugin\rest\resource\FileUploadResource::validate()
+    $filename = $matches['filename'];
+
+    // Make sure only the filename component is returned. Path information is
+    // stripped as per https://tools.ietf.org/html/rfc6266#section-4.3.
+    // We do not need to use Drupal's FileSystem service here as we are not
+    // dealing with StreamWrappers.
+    return \basename($filename);
+  }
+
+}

core/modules/file/tests/src/Unit/Upload/ContentDispositionFilenameParserTest.php

+<?php
+
+namespace Drupal\Tests\file\Unit\Upload;
+
+use Drupal\file\Upload\ContentDispositionFilenameParser;
+use Drupal\Tests\UnitTestCase;
+use Symfony\Component\HttpFoundation\Request;
+use Symfony\Component\HttpKernel\Exception\BadRequestHttpException;
+
+/**
+ * Tests the ContentDispositionFilenameParser class.
+ *
+ * @group file
+ * @coversDefaultClass \Drupal\file\Upload\ContentDispositionFilenameParser
+ */
+class ContentDispositionFilenameParserTest extends UnitTestCase {
+
+  /**
+   * Tests the parseFilename() method.
+   *
+   * @covers ::parseFilename
+   */
+  public function testParseFilenameSuccess(): void {
+    $request = $this->createRequest('filename="test.txt"');
+    $filename = ContentDispositionFilenameParser::parseFilename($request);
+    $this->assertEquals('test.txt', $filename);
+  }
+
+  /**
+   * @covers ::parseFilename
+   * @dataProvider invalidHeaderProvider
+   */
+  public function testParseFilenameInvalid(string | bool $contentDisposition): void {
+    $this->expectException(BadRequestHttpException::class);
+    $this->expectExceptionMessage('No filename found in "Content-Disposition" header. A file name in the format "filename=FILENAME" must be provided.');
+    $request = $this->createRequest($contentDisposition);
+    ContentDispositionFilenameParser::parseFilename($request);
+  }
+
+  /**
+   * @covers ::parseFilename
+   */
+  public function testParseFilenameMissing(): void {
+    $this->expectException(BadRequestHttpException::class);
+    $this->expectExceptionMessage('"Content-Disposition" header is required. A file name in the format "filename=FILENAME" must be provided.');
+    $request = new Request();
+    ContentDispositionFilenameParser::parseFilename($request);
+  }
+
+  /**
+   * @covers ::parseFilename
+   */
+  public function testParseFilenameExtended(): void {
+    $this->expectException(BadRequestHttpException::class);
+    $this->expectExceptionMessage('The extended "filename*" format is currently not supported in the "Content-Disposition" header.');
+    $request = $this->createRequest('filename*="UTF-8 \' \' example.txt"');
+    $filename = ContentDispositionFilenameParser::parseFilename($request);
+  }
+
+  /**
+   * A data provider for invalid headers.
+   */
+  public function invalidHeaderProvider(): array {
+    return [
+      'multiple' => ['file; filename=""'],
+      'empty' => ['filename=""'],
+      'bad key' => ['not_a_filename="example.txt"'],
+    ];
+  }
+
+  /**
+   * Creates a request with the given content-disposition header.
+   */
+  protected function createRequest(string $contentDisposition): Request {
+    $request = new Request();
+    $request->headers->set('Content-Disposition', $contentDisposition);
+    return $request;
+  }
+
+}

core/modules/jsonapi/src/Controller/FileUpload.php

@@ -10,6 +10,7 @@
 use Drupal\Core\Field\FieldDefinitionInterface;
 use Drupal\Core\Session\AccountInterface;
 use Drupal\Core\Url;
+use Drupal\file\Upload\ContentDispositionFilenameParser;
 use Drupal\jsonapi\Entity\EntityValidationTrait;
 use Drupal\jsonapi\JsonApiResource\JsonApiDocumentTopLevel;
 use Drupal\jsonapi\JsonApiResource\Link;
@@ -117,7 +118,7 @@ public function handleFileUploadForExistingResource(Request $request, ResourceTy
 
     static::ensureFileUploadAccess($this->currentUser, $field_definition, $entity);
 
-    $filename = $this->fileUploader->validateAndParseContentDispositionHeader($request);
+    $filename = ContentDispositionFilenameParser::parseFilename($request);
     $file = $this->fileUploader->handleFileUploadForField($field_definition, $filename, $this->currentUser);
 
     if ($file instanceof ConstraintViolationListInterface) {
@@ -167,7 +168,7 @@ public function handleFileUploadForNewResource(Request $request, ResourceType $r
 
     static::ensureFileUploadAccess($this->currentUser, $field_definition);
 
-    $filename = $this->fileUploader->validateAndParseContentDispositionHeader($request);
+    $filename = ContentDispositionFilenameParser::parseFilename($request);
     $file = $this->fileUploader->handleFileUploadForField($field_definition, $filename, $this->currentUser);
 
     if ($file instanceof ConstraintViolationListInterface) {

core/modules/jsonapi/src/Controller/TemporaryJsonapiFileFieldUploader.php

@@ -19,6 +19,7 @@
 use Drupal\file\Entity\File;
 use Drupal\file\FileInterface;
 use Drupal\file\Plugin\Field\FieldType\FileFieldItemList;
+use Drupal\file\Upload\ContentDispositionFilenameParser;
 use Drupal\file\Upload\InputStreamFileWriterInterface;
 use Drupal\file\Validation\FileValidatorInterface;
 use Psr\Log\LoggerInterface;
@@ -26,7 +27,6 @@
 use Symfony\Component\HttpFoundation\File\Exception\NoFileException;
 use Symfony\Component\HttpFoundation\File\Exception\UploadException;
 use Symfony\Component\HttpFoundation\Request;
-use Symfony\Component\HttpKernel\Exception\BadRequestHttpException;
 use Symfony\Component\HttpKernel\Exception\HttpException;
 use Symfony\Contracts\EventDispatcher\EventDispatcherInterface;
 
@@ -49,6 +49,12 @@ class TemporaryJsonapiFileFieldUploader {
    * The regex used to extract the filename from the content disposition header.
    *
    * @var string
+   *
+   * @deprecated in drupal:10.3.0 and is removed from drupal:11.0.0. Use
+   *   \Drupal\file\Upload\ContentDispositionFilenameParser::REQUEST_HEADER_FILENAME_REGEX
+   *   instead.
+   *
+   * @see https://www.drupal.org/node/3380380
    */
   const REQUEST_HEADER_FILENAME_REGEX = '@\bfilename(?<star>\*?)=\"(?<filename>.+)\"@';
 
@@ -276,35 +282,16 @@ public function handleFileUploadForField(FieldDefinitionInterface $field_definit
    *
    * @throws \Symfony\Component\HttpKernel\Exception\BadRequestHttpException
    *   Thrown when the 'Content-Disposition' request header is invalid.
+   *
+   * @deprecated in drupal:10.3.0 and is removed from drupal:11.0.0. Use
+   *   \Drupal\file\Upload\ContentDispositionFilenameParser::parseFilename()
+   *   instead.
+   *
+   * @see https://www.drupal.org/node/3380380
    */
   public function validateAndParseContentDispositionHeader(Request $request) {
-    // First, check the header exists.
-    if (!$request->headers->has('content-disposition')) {
-      throw new BadRequestHttpException('"Content-Disposition" header is required. A file name in the format "filename=FILENAME" must be provided.');
-    }
-
-    $content_disposition = $request->headers->get('content-disposition');
-
-    // Parse the header value. This regex does not allow an empty filename.
-    // i.e. 'filename=""'. This also matches on a word boundary so other keys
-    // like 'not_a_filename' don't work.
-    if (!preg_match(static::REQUEST_HEADER_FILENAME_REGEX, $content_disposition, $matches)) {
-      throw new BadRequestHttpException('No filename found in "Content-Disposition" header. A file name in the format "filename=FILENAME" must be provided.');
-    }
-
-    // Check for the "filename*" format. This is currently unsupported.
-    if (!empty($matches['star'])) {
-      throw new BadRequestHttpException('The extended "filename*" format is currently not supported in the "Content-Disposition" header.');
-    }
-
-    // Don't validate the actual filename here, that will be done by the upload
-    // validators in validate().
-    // @see \Drupal\file\Plugin\rest\resource\FileUploadResource::validate()
-    $filename = $matches['filename'];
-
-    // Make sure only the filename component is returned. Path information is
-    // stripped as per https://tools.ietf.org/html/rfc6266#section-4.3.
-    return $this->fileSystem->basename($filename);
+    @trigger_error('Calling ' . __METHOD__ . '() is deprecated in drupal:10.3.0 and is removed from drupal:11.0.0. Use \Drupal\file\Upload\ContentDispositionFilenameParser::parseFilename() instead. See https://www.drupal.org/node/3380380', E_USER_DEPRECATED);
+    return ContentDispositionFilenameParser::parseFilename($request);
   }
 
   /**

core/modules/rest/tests/src/Functional/FileUploadResourceTestBase.php

@@ -267,26 +267,26 @@ public function testPostFileUploadInvalidHeaders() {
 
     // An empty Content-Disposition header should return a 400.
     $response = $this->fileRequest($uri, $this->testFileData, ['Content-Disposition' => FALSE]);
-    $this->assertResourceErrorResponse(400, '"Content-Disposition" header is required. A file name in the format "filename=FILENAME" must be provided', $response);
+    $this->assertResourceErrorResponse(400, '"Content-Disposition" header is required. A file name in the format "filename=FILENAME" must be provided.', $response);
 
     // An empty filename with a context in the Content-Disposition header should
     // return a 400.
     $response = $this->fileRequest($uri, $this->testFileData, ['Content-Disposition' => 'file; filename=""']);
-    $this->assertResourceErrorResponse(400, 'No filename found in "Content-Disposition" header. A file name in the format "filename=FILENAME" must be provided', $response);
+    $this->assertResourceErrorResponse(400, 'No filename found in "Content-Disposition" header. A file name in the format "filename=FILENAME" must be provided.', $response);
 
     // An empty filename without a context in the Content-Disposition header
     // should return a 400.
     $response = $this->fileRequest($uri, $this->testFileData, ['Content-Disposition' => 'filename=""']);
-    $this->assertResourceErrorResponse(400, 'No filename found in "Content-Disposition" header. A file name in the format "filename=FILENAME" must be provided', $response);
+    $this->assertResourceErrorResponse(400, 'No filename found in "Content-Disposition" header. A file name in the format "filename=FILENAME" must be provided.', $response);
 
     // An invalid key-value pair in the Content-Disposition header should return
     // a 400.
     $response = $this->fileRequest($uri, $this->testFileData, ['Content-Disposition' => 'not_a_filename="example.txt"']);
-    $this->assertResourceErrorResponse(400, 'No filename found in "Content-Disposition" header. A file name in the format "filename=FILENAME" must be provided', $response);
+    $this->assertResourceErrorResponse(400, 'No filename found in "Content-Disposition" header. A file name in the format "filename=FILENAME" must be provided.', $response);
 
     // Using filename* extended format is not currently supported.
     $response = $this->fileRequest($uri, $this->testFileData, ['Content-Disposition' => 'filename*="UTF-8 \' \' example.txt"']);
-    $this->assertResourceErrorResponse(400, 'The extended "filename*" format is currently not supported in the "Content-Disposition" header', $response);
+    $this->assertResourceErrorResponse(400, 'The extended "filename*" format is currently not supported in the "Content-Disposition" header.', $response);
   }
 
   /**