From 4d6f38f68ce1ac483b4b1e3d2330dc09c50454c3 Mon Sep 17 00:00:00 2001 From: Lee Rowlands <lee.rowlands@previousnext.com.au> Date: Mon, 13 Nov 2023 11:30:17 +1000 Subject: [PATCH] Issue #3380345 by kim.pepper, smustgrave, quietone, larowlan: Create a InputStreamFileWriter for writing the input stream to a file --- core/modules/file/file.services.yml | 4 + .../rest/resource/FileUploadResource.php | 76 +++++++++---------- .../file/src/Upload/InputStreamFileWriter.php | 67 ++++++++++++++++ .../Upload/InputStreamFileWriterInterface.php | 34 +++++++++ .../Kernel/Upload/StreamFileUploaderTest.php | 59 ++++++++++++++ core/modules/jsonapi/jsonapi.services.yml | 2 +- .../TemporaryJsonapiFileFieldUploader.php | 75 ++++++++---------- 7 files changed, 230 insertions(+), 87 deletions(-) create mode 100644 core/modules/file/src/Upload/InputStreamFileWriter.php create mode 100644 core/modules/file/src/Upload/InputStreamFileWriterInterface.php create mode 100644 core/modules/file/tests/src/Kernel/Upload/StreamFileUploaderTest.php diff --git a/core/modules/file/file.services.yml b/core/modules/file/file.services.yml index 7dc90da042b6..20df92114713 100644 --- a/core/modules/file/file.services.yml +++ b/core/modules/file/file.services.yml @@ -32,3 +32,7 @@ services: class: Drupal\file\Validation\UploadedFileValidator arguments: ['@validation.basic_recursive_validator_factory'] Drupal\file\Validation\UploadedFileValidatorInterface: '@file.uploaded_file_validator' + file.input_stream_file_writer: + class: Drupal\file\Upload\InputStreamFileWriter + arguments: ['@file_system'] + Drupal\file\Upload\InputStreamFileWriterInterface: '@file.input_stream_file_writer' diff --git a/core/modules/file/src/Plugin/rest/resource/FileUploadResource.php b/core/modules/file/src/Plugin/rest/resource/FileUploadResource.php index 9ae8c89f8820..84d346fcf053 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\InputStreamFileWriterInterface; use Drupal\file\Validation\FileValidatorInterface; use Drupal\rest\ModifiedResourceResponse; use Drupal\rest\Plugin\ResourceBase; @@ -24,6 +25,9 @@ use Drupal\rest\RequestHandler; use Psr\Log\LoggerInterface; use Symfony\Component\DependencyInjection\ContainerInterface; +use Symfony\Component\HttpFoundation\File\Exception\CannotWriteFileException; +use Symfony\Component\HttpFoundation\File\Exception\NoFileException; +use Symfony\Component\HttpFoundation\File\Exception\UploadException; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpKernel\Exception\AccessDeniedHttpException; use Symfony\Component\HttpKernel\Exception\BadRequestHttpException; @@ -141,6 +145,11 @@ class FileUploadResource extends ResourceBase { */ protected FileValidatorInterface $fileValidator; + /** + * The input stream file writer. + */ + protected InputStreamFileWriterInterface $inputStreamFileWriter; + /** * Constructs a FileUploadResource instance. * @@ -174,8 +183,10 @@ class FileUploadResource extends ResourceBase { * The event dispatcher service. * @param \Drupal\file\Validation\FileValidatorInterface|null $file_validator * The file validator service. + * @param \Drupal\file\Upload\InputStreamFileWriterInterface|null $input_stream_file_writer + * The input stream file writer. */ - public function __construct(array $configuration, $plugin_id, $plugin_definition, $serializer_formats, LoggerInterface $logger, FileSystemInterface $file_system, EntityTypeManagerInterface $entity_type_manager, EntityFieldManagerInterface $entity_field_manager, AccountInterface $current_user, $mime_type_guesser, Token $token, LockBackendInterface $lock, Config $system_file_config, EventDispatcherInterface $event_dispatcher, FileValidatorInterface $file_validator = NULL) { + public function __construct(array $configuration, $plugin_id, $plugin_definition, $serializer_formats, LoggerInterface $logger, FileSystemInterface $file_system, EntityTypeManagerInterface $entity_type_manager, EntityFieldManagerInterface $entity_field_manager, AccountInterface $current_user, $mime_type_guesser, Token $token, LockBackendInterface $lock, Config $system_file_config, EventDispatcherInterface $event_dispatcher, FileValidatorInterface $file_validator = NULL, InputStreamFileWriterInterface $input_stream_file_writer = NULL) { parent::__construct($configuration, $plugin_id, $plugin_definition, $serializer_formats, $logger); $this->fileSystem = $file_system; $this->entityTypeManager = $entity_type_manager; @@ -191,6 +202,11 @@ public function __construct(array $configuration, $plugin_id, $plugin_definition $file_validator = \Drupal::service('file.validator'); } $this->fileValidator = $file_validator; + if (!$input_stream_file_writer) { + @trigger_error('Calling ' . __METHOD__ . '() without the $input_stream_file_writer argument is deprecated in drupal:10.3.0 and is required in drupal:11.0.0. See https://www.drupal.org/node/123', E_USER_DEPRECATED); + $input_stream_file_writer = \Drupal::service('file.input_stream_file_writer'); + } + $this->inputStreamFileWriter = $input_stream_file_writer; } /** @@ -212,7 +228,8 @@ public static function create(ContainerInterface $container, array $configuratio $container->get('lock'), $container->get('config.factory')->get('system.file'), $container->get('event_dispatcher'), - $container->get('file.validator') + $container->get('file.validator'), + $container->get('file.input_stream_file_writer') ); } @@ -341,48 +358,23 @@ public function post(Request $request, $entity_type_id, $bundle, $field_name) { * Thrown when input data cannot be read, the temporary file cannot be * opened, or the temporary file cannot be written. */ - protected function streamUploadData() { - // 'rb' is needed so reading works correctly on Windows environments too. - $file_data = fopen('php://input', 'rb'); - - $temp_file_path = $this->fileSystem->tempnam('temporary://', 'file'); - $temp_file = fopen($temp_file_path, 'wb'); - - if ($temp_file) { - while (!feof($file_data)) { - $read = fread($file_data, static::BYTES_TO_READ); - - if ($read === FALSE) { - // Close the file streams. - fclose($temp_file); - fclose($file_data); - $this->logger->error('Input data could not be read'); - throw new HttpException(500, 'Input file data could not be read'); - } - - if (fwrite($temp_file, $read) === FALSE) { - // Close the file streams. - fclose($temp_file); - fclose($file_data); - $this->logger->error('Temporary file data for "%path" could not be written', ['%path' => $temp_file_path]); - throw new HttpException(500, 'Temporary file data could not be written'); - } - } - - // Close the temp file stream. - fclose($temp_file); + protected function streamUploadData(): string { + // Catch and throw the exceptions that REST expects. + try { + $temp_file_path = $this->inputStreamFileWriter->writeStreamToFile(); } - else { - // Close the input file stream since we can't proceed with the upload. - // Don't try to close $temp_file since it's FALSE at this point. - fclose($file_data); - $this->logger->error('Temporary file "%path" could not be opened for file upload', ['%path' => $temp_file_path]); - throw new HttpException(500, 'Temporary file could not be opened'); + catch (UploadException $e) { + $this->logger->error('Input data could not be read'); + throw new HttpException(500, 'Input file data could not be read', $e); + } + catch (CannotWriteFileException $e) { + $this->logger->error('Temporary file data for could not be written'); + throw new HttpException(500, 'Temporary file data could not be written', $e); + } + catch (NoFileException $e) { + $this->logger->error('Temporary file could not be opened for file upload'); + throw new HttpException(500, 'Temporary file could not be opened', $e); } - - // Close the input stream. - fclose($file_data); - return $temp_file_path; } diff --git a/core/modules/file/src/Upload/InputStreamFileWriter.php b/core/modules/file/src/Upload/InputStreamFileWriter.php new file mode 100644 index 000000000000..a5afaac16b50 --- /dev/null +++ b/core/modules/file/src/Upload/InputStreamFileWriter.php @@ -0,0 +1,67 @@ +<?php + +namespace Drupal\file\Upload; + +use Drupal\Core\File\FileSystemInterface; +use Symfony\Component\HttpFoundation\File\Exception\CannotWriteFileException; +use Symfony\Component\HttpFoundation\File\Exception\NoFileException; +use Symfony\Component\HttpFoundation\File\Exception\UploadException; + +/** + * Writes files from a input stream to a temporary file. + */ +class InputStreamFileWriter implements InputStreamFileWriterInterface { + + /** + * Creates a new InputStreamFileUploader. + */ + public function __construct( + protected FileSystemInterface $fileSystem, + ) {} + + /** + * {@inheritdoc} + */ + public function writeStreamToFile(string $stream = self::DEFAULT_STREAM, int $bytesToRead = self::DEFAULT_BYTES_TO_READ): string { + // 'rb' is needed so reading works correctly on Windows environments too. + $fileData = fopen($stream, 'rb'); + + $tempFilePath = $this->fileSystem->tempnam('temporary://', 'file'); + $tempFile = fopen($tempFilePath, 'wb'); + + if ($tempFile) { + while (!feof($fileData)) { + $read = fread($fileData, $bytesToRead); + + if ($read === FALSE) { + // Close the file streams. + fclose($tempFile); + fclose($fileData); + throw new UploadException('Input file data could not be read'); + } + + if (fwrite($tempFile, $read) === FALSE) { + // Close the file streams. + fclose($tempFile); + fclose($fileData); + throw new CannotWriteFileException(sprintf('Temporary file data for "%s" could not be written', $tempFilePath)); + } + } + + // Close the temp file stream. + fclose($tempFile); + } + else { + // Close the input file stream since we can't proceed with the upload. + // Don't try to close $tempFile since it's FALSE at this point. + fclose($fileData); + throw new NoFileException(sprintf('Temporary file "%s" could not be opened for file upload', $tempFilePath)); + } + + // Close the input stream. + fclose($fileData); + + return $tempFilePath; + } + +} diff --git a/core/modules/file/src/Upload/InputStreamFileWriterInterface.php b/core/modules/file/src/Upload/InputStreamFileWriterInterface.php new file mode 100644 index 000000000000..f30c18c77188 --- /dev/null +++ b/core/modules/file/src/Upload/InputStreamFileWriterInterface.php @@ -0,0 +1,34 @@ +<?php + +namespace Drupal\file\Upload; + +/** + * Uploads files from a stream. + */ +interface InputStreamFileWriterInterface { + + + /** + * The length of bytes to read in each iteration when streaming file data. + */ + const DEFAULT_BYTES_TO_READ = 8192; + + /** + * The default stream. + */ + const DEFAULT_STREAM = "php://input"; + + /** + * Write the input stream to a temporary file. + * + * @param string $stream + * (optional) The input stream. + * @param int $bytesToRead + * (optional) The length of bytes to read in each iteration. + * + * @return string + * The temporary file path. + */ + public function writeStreamToFile(string $stream = self::DEFAULT_STREAM, int $bytesToRead = self::DEFAULT_BYTES_TO_READ): string; + +} diff --git a/core/modules/file/tests/src/Kernel/Upload/StreamFileUploaderTest.php b/core/modules/file/tests/src/Kernel/Upload/StreamFileUploaderTest.php new file mode 100644 index 000000000000..aadba75dc753 --- /dev/null +++ b/core/modules/file/tests/src/Kernel/Upload/StreamFileUploaderTest.php @@ -0,0 +1,59 @@ +<?php + +declare(strict_types=1); + +namespace Drupal\Tests\file\Kernel\Upload; + +use Drupal\KernelTests\KernelTestBase; +use org\bovigo\vfs\vfsStream; + +/** + * Tests the stream file uploader. + * + * @group file + * @coversDefaultClass \Drupal\file\Upload\InputStreamFileWriter + */ +class StreamFileUploaderTest extends KernelTestBase { + + /** + * {@inheritdoc} + */ + protected static $modules = ['file']; + + /** + * @covers ::writeStreamToFile + */ + public function testWriteStreamToFileSuccess(): void { + vfsStream::newFile('foo.txt') + ->at($this->vfsRoot) + ->withContent('bar'); + + $fileWriter = $this->container->get('file.input_stream_file_writer'); + + $filename = $fileWriter->writeStreamToFile(vfsStream::url('root/foo.txt')); + + $this->assertStringStartsWith('temporary://', $filename); + $this->assertStringEqualsFile($filename, 'bar'); + } + + /** + * @covers ::writeStreamToFile + */ + public function testWriteStreamToFileWithSmallerBytes(): void { + $content = $this->randomString(2048); + vfsStream::newFile('foo.txt') + ->at($this->vfsRoot) + ->withContent($content); + + $fileWriter = $this->container->get('file.input_stream_file_writer'); + + $filename = $fileWriter->writeStreamToFile( + stream: vfsStream::url('root/foo.txt'), + bytesToRead: 1024, + ); + + $this->assertStringStartsWith('temporary://', $filename); + $this->assertStringEqualsFile($filename, $content); + } + +} diff --git a/core/modules/jsonapi/jsonapi.services.yml b/core/modules/jsonapi/jsonapi.services.yml index e6900aa62885..1397335257c2 100644 --- a/core/modules/jsonapi/jsonapi.services.yml +++ b/core/modules/jsonapi/jsonapi.services.yml @@ -261,5 +261,5 @@ services: jsonapi.file.uploader.field: class: Drupal\jsonapi\Controller\TemporaryJsonapiFileFieldUploader public: false - arguments: ['@logger.channel.file', '@file_system', '@file.mime_type.guesser', '@token', '@lock', '@config.factory', '@event_dispatcher', '@file.validator'] + arguments: ['@logger.channel.file', '@file_system', '@file.mime_type.guesser', '@token', '@lock', '@config.factory', '@event_dispatcher', '@file.validator', '@file.input_stream_file_writer'] Drupal\jsonapi\Controller\TemporaryJsonapiFileFieldUploader: '@jsonapi.file.uploader.field' diff --git a/core/modules/jsonapi/src/Controller/TemporaryJsonapiFileFieldUploader.php b/core/modules/jsonapi/src/Controller/TemporaryJsonapiFileFieldUploader.php index 8ecf3d999fce..3b4934f78101 100644 --- a/core/modules/jsonapi/src/Controller/TemporaryJsonapiFileFieldUploader.php +++ b/core/modules/jsonapi/src/Controller/TemporaryJsonapiFileFieldUploader.php @@ -19,8 +19,12 @@ use Drupal\file\Entity\File; use Drupal\file\FileInterface; use Drupal\file\Plugin\Field\FieldType\FileFieldItemList; +use Drupal\file\Upload\InputStreamFileWriterInterface; use Drupal\file\Validation\FileValidatorInterface; use Psr\Log\LoggerInterface; +use Symfony\Component\HttpFoundation\File\Exception\CannotWriteFileException; +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; @@ -111,6 +115,11 @@ class TemporaryJsonapiFileFieldUploader { */ protected FileValidatorInterface $fileValidator; + /** + * The input stream file writer. + */ + protected InputStreamFileWriterInterface $inputStreamFileWriter; + /** * Constructs a FileUploadResource instance. * @@ -126,12 +135,14 @@ class TemporaryJsonapiFileFieldUploader { * The lock service. * @param \Drupal\Core\Config\ConfigFactoryInterface $config_factory * The config factory. - * @param \Symfony\Contracts\EventDispatcher\EventDispatcherInterface $event_dispatcher + * @param \Symfony\Contracts\EventDispatcher\EventDispatcherInterface|null $event_dispatcher * (optional) The event dispatcher. * @param \Drupal\file\Validation\FileValidatorInterface|null $file_validator * The file validator. + * @param \Drupal\file\Upload\InputStreamFileWriterInterface|null $input_stream_file_writer + * The stream file uploader. */ - public function __construct(LoggerInterface $logger, FileSystemInterface $file_system, $mime_type_guesser, Token $token, LockBackendInterface $lock, ConfigFactoryInterface $config_factory, EventDispatcherInterface $event_dispatcher = NULL, FileValidatorInterface $file_validator = NULL) { + public function __construct(LoggerInterface $logger, FileSystemInterface $file_system, $mime_type_guesser, Token $token, LockBackendInterface $lock, ConfigFactoryInterface $config_factory, EventDispatcherInterface $event_dispatcher = NULL, FileValidatorInterface $file_validator = NULL, InputStreamFileWriterInterface $input_stream_file_writer = NULL) { $this->logger = $logger; $this->fileSystem = $file_system; $this->mimeTypeGuesser = $mime_type_guesser; @@ -147,6 +158,11 @@ public function __construct(LoggerInterface $logger, FileSystemInterface $file_s $file_validator = \Drupal::service('file.validator'); } $this->fileValidator = $file_validator; + if (!$input_stream_file_writer) { + @trigger_error('Calling ' . __METHOD__ . '() without the $input_stream_file_writer argument is deprecated in drupal:10.3.0 and is required in drupal:11.0.0. See https://www.drupal.org/node/123', E_USER_DEPRECATED); + $input_stream_file_writer = \Drupal::service('file.input_stream_file_writer'); + } + $this->inputStreamFileWriter = $input_stream_file_writer; } /** @@ -333,51 +349,22 @@ public static function checkFileUploadAccess(AccountInterface $account, FieldDef * opened, or the temporary file cannot be written. */ protected function streamUploadData() { - // 'rb' is needed so reading works correctly on Windows environments too. - $file_data = fopen('php://input', 'rb'); - - $temp_file_path = $this->fileSystem->tempnam('temporary://', 'file'); - if ($temp_file_path === FALSE) { - $this->logger->error('Temporary file could not be created for file upload.'); - throw new HttpException(500, 'Temporary file could not be created'); + // Catch and throw the exceptions that JSON API module expects. + try { + $temp_file_path = $this->inputStreamFileWriter->writeStreamToFile(); } - $temp_file = fopen($temp_file_path, 'wb'); - - if ($temp_file) { - while (!feof($file_data)) { - $read = fread($file_data, static::BYTES_TO_READ); - - if ($read === FALSE) { - // Close the file streams. - fclose($temp_file); - fclose($file_data); - $this->logger->error('Input data could not be read'); - throw new HttpException(500, 'Input file data could not be read.'); - } - - if (fwrite($temp_file, $read) === FALSE) { - // Close the file streams. - fclose($temp_file); - fclose($file_data); - $this->logger->error('Temporary file data for "%path" could not be written', ['%path' => $temp_file_path]); - throw new HttpException(500, 'Temporary file data could not be written.'); - } - } - - // Close the temp file stream. - fclose($temp_file); + catch (UploadException $e) { + $this->logger->error('Input data could not be read'); + throw new HttpException(500, 'Input file data could not be read', $e); } - else { - // Close the input file stream since we can't proceed with the upload. - // Don't try to close $temp_file since it's FALSE at this point. - fclose($file_data); - $this->logger->error('Temporary file "%path" could not be opened for file upload.', ['%path' => $temp_file_path]); - throw new HttpException(500, 'Temporary file could not be opened'); + catch (CannotWriteFileException $e) { + $this->logger->error('Temporary file data for could not be written'); + throw new HttpException(500, 'Temporary file data could not be written', $e); + } + catch (NoFileException $e) { + $this->logger->error('Temporary file could not be opened for file upload'); + throw new HttpException(500, 'Temporary file could not be opened', $e); } - - // Close the input stream. - fclose($file_data); - return $temp_file_path; } -- GitLab