diff --git a/core/modules/file/src/Plugin/rest/resource/FileUploadResource.php b/core/modules/file/src/Plugin/rest/resource/FileUploadResource.php index 211de1f58b..22e2abfb4f 100644 --- a/core/modules/file/src/Plugin/rest/resource/FileUploadResource.php +++ b/core/modules/file/src/Plugin/rest/resource/FileUploadResource.php @@ -3,8 +3,10 @@ namespace Drupal\file\Plugin\rest\resource; use Drupal\Component\Utility\Bytes; +use Drupal\Component\Utility\Crypt; use Drupal\Core\Entity\EntityTypeManagerInterface; use Drupal\Core\Field\FieldDefinitionInterface; +use Drupal\Core\Lock\LockBackendInterface; use Drupal\Core\Session\AccountInterface; use Drupal\Core\Utility\Token; use Drupal\file\FileInterface; @@ -82,10 +84,15 @@ class FileUploadResource extends ResourceBase { protected $mimeTypeGuesser; /** - * @var \Drupal\Core\Utility\Token|Token + * @var \Drupal\Core\Utility\Token */ protected $token; + /** + * @var \Drupal\Core\Lock\LockBackendInterface + */ + protected $lock; + /** * Constructs a FileUploadResource instance. * @@ -113,7 +120,7 @@ class FileUploadResource extends ResourceBase { * The token replacement instance. * */ - public function __construct(array $configuration, $plugin_id, $plugin_definition, $serializer_formats, LoggerInterface $logger, FileSystem $file_system, EntityTypeManagerInterface $entity_type_manager, EntityFieldManagerInterface $entity_field_manager, AccountInterface $current_user, MimeTypeGuesserInterface $mime_type_guesser, Token $token) { + public function __construct(array $configuration, $plugin_id, $plugin_definition, $serializer_formats, LoggerInterface $logger, FileSystem $file_system, EntityTypeManagerInterface $entity_type_manager, EntityFieldManagerInterface $entity_field_manager, AccountInterface $current_user, MimeTypeGuesserInterface $mime_type_guesser, Token $token, LockBackendInterface $lock) { parent::__construct($configuration, $plugin_id, $plugin_definition, $serializer_formats, $logger); $this->fileSystem = $file_system; $this->entityTypeManager = $entity_type_manager; @@ -121,6 +128,7 @@ public function __construct(array $configuration, $plugin_id, $plugin_definition $this->currentUser = $current_user; $this->mimeTypeGuesser = $mime_type_guesser; $this->token = $token; + $this->lock = $lock; } /** @@ -138,7 +146,8 @@ public static function create(ContainerInterface $container, array $configuratio $container->get('entity_field.manager'), $container->get('current_user'), $container->get('file.mime_type.guesser'), - $container->get('token') + $container->get('token'), + $container->get('lock') ); } @@ -156,6 +165,8 @@ public static function create(ContainerInterface $container, array $configuratio * * @return \Drupal\rest\ModifiedResourceResponse * A 201 response, on success. + * + * @throws \Symfony\Component\HttpKernel\Exception\HttpException */ public function post(Request $request, $entity_type_id, $bundle, $field_name) { $filename = $this->validateAndParseContentDispositionHeader($request); @@ -177,6 +188,13 @@ public function post(Request $request, $entity_type_id, $bundle, $field_name) { // This will take care of altering $file_uri if a file already exists. file_unmanaged_prepare($temp_file_path, $file_uri); + // Lock based on the prepared file URI. + $lock_id = $this->generateLockIdFromFileUri($file_uri); + + if (!$this->lock->acquire($lock_id)) { + throw new HttpException(500, sprintf('File "%s" is already locked for writing')); + } + // Begin building file entity. $values = [ 'uid' => $this->currentUser->id(), @@ -202,6 +220,8 @@ public function post(Request $request, $entity_type_id, $bundle, $field_name) { $file->save(); + $this->lock->release($lock_id); + // 201 Created responses return the newly created entity in the response // body. These responses are not cacheable, so we add no cacheability // metadata here. @@ -226,19 +246,36 @@ protected function streamUploadData() { $file_data = fopen('php://input','rb'); $temp_file_path = $this->fileSystem->tempnam('temporary://', 'file'); + $temp_file = fopen($temp_file_path, 'wb'); - if ($temp_file = fopen($temp_file_path, 'wb')) { + if ($temp_file) { while (!feof($file_data)) { - fwrite($temp_file, fread($file_data, 8192)); + $read = fread($file_data, 8192); + + if ($read === FALSE) { + // Close the temp file stream. + fclose($temp_file); + $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 temp file stream. + fclose($temp_file); + $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); } else { - $this->getLogger('file system')->error('Temporary file "%path" could not be opened for file upload', ['%path' => $temp_file_path]); + $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'); } + // Close the input stream. fclose($file_data); return $temp_file_path; @@ -275,7 +312,8 @@ protected function validateAndParseContentDispositionHeader(Request $request) { // @see file_validate() $filename = $matches['filename']; - return $filename; + // Make sure only the trailing filename is returned. + return basename($filename); } /** @@ -421,5 +459,17 @@ protected function getBaseRouteRequirements($method) { return $requirements; } + /** + * Generates a lock ID based on the file URI. + * + * @param $file_uri + * THe file URI. + * + * @return string + * The generated lock ID. + */ + protected function generateLockIdFromFileUri($file_uri) { + return 'file:rest:' . Crypt::hashBase64($file_uri); + } } diff --git a/core/modules/rest/src/RequestHandler.php b/core/modules/rest/src/RequestHandler.php index 3de5839d3a..ab65047284 100644 --- a/core/modules/rest/src/RequestHandler.php +++ b/core/modules/rest/src/RequestHandler.php @@ -209,8 +209,8 @@ protected function denormalizeRequestData(Request $request, ResourceInterface $r try { $unserialized = $serializer->denormalize($unserialized, $definition['serialization_class'], $format, ['request_method' => $method]); } - // These two serialization exception types mean there was a problem - // with the structure of the decoded data and it's not valid. + // These two serialization exception types mean there was a problem + // with the structure of the decoded data and it's not valid. catch (UnexpectedValueException $e) { throw new UnprocessableEntityHttpException($e->getMessage()); } diff --git a/core/modules/rest/tests/src/Functional/FileUploadResourceTestBase.php b/core/modules/rest/tests/src/Functional/FileUploadResourceTestBase.php index 9b3d683e09..920f3e78b2 100644 --- a/core/modules/rest/tests/src/Functional/FileUploadResourceTestBase.php +++ b/core/modules/rest/tests/src/Functional/FileUploadResourceTestBase.php @@ -209,6 +209,30 @@ public function testPostFileUploadDuplicateFile() { $this->assertSame($this->testFileData, file_get_contents('public://foobar/example_0.txt')); } + /** + * Tests using the file upload route with any path prefixes being stripped. + */ + public function testFileUploadStrippedFilePath() { + $this->initAuthentication(); + + $this->provisionResource([static::$format], static::$auth ? [static::$auth] : [], ['POST']); + + $this->setUpAuthorization('POST'); + + $uri = Url::fromUri('base:' . static::$postUri); + + $response = $this->fileRequest($uri, $this->testFileData, ['Content-Disposition' => 'file; filename="directory/example.txt"']); + + $this->assertSame(201, $response->getStatusCode()); + + $expected = $this->getExpectedNormalizedEntity(); + $this->assertResponseData($expected, $response); + + // Check the actual file data. It should have been written to the configured + // directory, not /foobar/directory/example.txt. + $this->assertSame($this->testFileData, file_get_contents('public://foobar/example.txt')); + } + /** * Tests using the file upload route with a zero byte file. */