From 56b614e7e62c368672daa21daca792039462cfbc Mon Sep 17 00:00:00 2001
From: Lee Rowlands <lee.rowlands@previousnext.com.au>
Date: Mon, 13 Nov 2023 12:52:19 +1000
Subject: [PATCH] Issue #3380379 by kim.pepper, smustgrave, quietone: Create
content disposition filename extractor and deprecate duplicate code in REST
and JSON API
---
.../rest/resource/FileUploadResource.php | 45 ++++-------
.../ContentDispositionFilenameParser.php | 67 ++++++++++++++++
.../ContentDispositionFilenameParserTest.php | 80 +++++++++++++++++++
.../jsonapi/src/Controller/FileUpload.php | 5 +-
.../TemporaryJsonapiFileFieldUploader.php | 43 ++++------
.../Functional/FileUploadResourceTestBase.php | 10 +--
6 files changed, 186 insertions(+), 64 deletions(-)
create mode 100644 core/modules/file/src/Upload/ContentDispositionFilenameParser.php
create mode 100644 core/modules/file/tests/src/Unit/Upload/ContentDispositionFilenameParserTest.php
diff --git a/core/modules/file/src/Plugin/rest/resource/FileUploadResource.php b/core/modules/file/src/Plugin/rest/resource/FileUploadResource.php
index 84d346fcf053..8abc3cc29f13 100644
--- a/core/modules/file/src/Plugin/rest/resource/FileUploadResource.php
+++ b/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);
}
/**
diff --git a/core/modules/file/src/Upload/ContentDispositionFilenameParser.php b/core/modules/file/src/Upload/ContentDispositionFilenameParser.php
new file mode 100644
index 000000000000..c49038308ddc
--- /dev/null
+++ b/core/modules/file/src/Upload/ContentDispositionFilenameParser.php
@@ -0,0 +1,67 @@
+<?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);
+ }
+
+}
diff --git a/core/modules/file/tests/src/Unit/Upload/ContentDispositionFilenameParserTest.php b/core/modules/file/tests/src/Unit/Upload/ContentDispositionFilenameParserTest.php
new file mode 100644
index 000000000000..04c6f9d1f614
--- /dev/null
+++ b/core/modules/file/tests/src/Unit/Upload/ContentDispositionFilenameParserTest.php
@@ -0,0 +1,80 @@
+<?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;
+ }
+
+}
diff --git a/core/modules/jsonapi/src/Controller/FileUpload.php b/core/modules/jsonapi/src/Controller/FileUpload.php
index 4c85a3aabaad..38dacc64ddab 100644
--- a/core/modules/jsonapi/src/Controller/FileUpload.php
+++ b/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) {
diff --git a/core/modules/jsonapi/src/Controller/TemporaryJsonapiFileFieldUploader.php b/core/modules/jsonapi/src/Controller/TemporaryJsonapiFileFieldUploader.php
index 3b4934f78101..b897ed5dfb2b 100644
--- a/core/modules/jsonapi/src/Controller/TemporaryJsonapiFileFieldUploader.php
+++ b/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);
}
/**
diff --git a/core/modules/rest/tests/src/Functional/FileUploadResourceTestBase.php b/core/modules/rest/tests/src/Functional/FileUploadResourceTestBase.php
index 5e1afccfcdd3..cc47f39fb765 100644
--- a/core/modules/rest/tests/src/Functional/FileUploadResourceTestBase.php
+++ b/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);
}
/**
--
GitLab