diff --git a/core/modules/file/file.module b/core/modules/file/file.module index 98d7788c41666506f89bb494fc2142684aabf7d4..9153fac739d470fe77f420f5313a1fc565f836b2 100644 --- a/core/modules/file/file.module +++ b/core/modules/file/file.module @@ -995,6 +995,9 @@ function _file_save_upload_single(\SplFileInfo $file_info, $form_field_name, $va $values['filemime'] = \Drupal::service('file.mime_type.guesser')->guess($values['filename']); $file = File::create($values); + // Ensure that the file object is validated. + $file->setValidationRequired(TRUE); + $extensions = ''; if (isset($validators['file_validate_extensions'])) { if (isset($validators['file_validate_extensions'][0])) { @@ -1124,6 +1127,28 @@ function _file_save_upload_single(\SplFileInfo $file_info, $form_field_name, $va // due to an existing file. $file->setFilename(\Drupal::service('file_system')->basename($file->destination)); + // We can now validate the file object itself before it's saved. + $violations = $file->validate(); + foreach ($violations as $violation) { + $errors[] = $violation->getMessage(); + } + if (!empty($errors)) { + $message = [ + 'error' => [ + '#markup' => t('The specified file %name could not be uploaded.', ['%name' => $file->getFilename()]), + ], + 'item_list' => [ + '#theme' => 'item_list', + '#items' => $errors, + ], + ]; + // @todo Add support for render arrays in + // \Drupal\Core\Messenger\MessengerInterface::addMessage()? + // @see https://www.drupal.org/node/2505497. + \Drupal::messenger()->addError(\Drupal::service('renderer')->renderPlain($message)); + return FALSE; + } + // If we made it this far it's safe to record this file in the database. $file->save(); diff --git a/core/modules/file/tests/src/Functional/SaveUploadTest.php b/core/modules/file/tests/src/Functional/SaveUploadTest.php index dbc42232bd1259eb8b20e7c86f301169d452ac49..274f7aae8207e7ad2a9bcc251454b7c7e5eafe1b 100644 --- a/core/modules/file/tests/src/Functional/SaveUploadTest.php +++ b/core/modules/file/tests/src/Functional/SaveUploadTest.php @@ -134,6 +134,35 @@ public function testNormal() { $this->assertTrue(is_file('temporary://' . $dir . '/' . trim(\Drupal::service('file_system')->basename($image3_realpath)))); } + /** + * Test uploading a duplicate file. + */ + public function testDuplicate() { + // It should not be possible to create two managed files with the same URI. + $image1 = current($this->drupalGetTestFiles('image')); + $edit = ['files[file_test_upload]' => \Drupal::service('file_system')->realpath($image1->uri)]; + $this->drupalPostForm('file-test/upload', $edit, t('Submit')); + $max_fid_after = (int) \Drupal::entityQueryAggregate('file')->aggregate('fid', 'max')->execute()[0]['fid_max']; + $file1 = File::load($max_fid_after); + + // Simulate a race condition where two files are uploaded at almost the same + // time, by removing the first uploaded file from disk (leaving the entry in + // the file_managed table) before trying to upload another file with the + // same name. + unlink(\Drupal::service('file_system')->realpath($file1->getFileUri())); + + $image2 = $image1; + $edit = ['files[file_test_upload]' => \Drupal::service('file_system')->realpath($image2->uri)]; + $this->drupalPostForm('file-test/upload', $edit, t('Submit')); + // Received a 200 response for posted test file. + $this->assertResponse(200); + $message = t('The file %file already exists. Enter a unique file URI.', ['%file' => $file1->getFileUri()]); + $this->assertRaw($message); + $max_fid_before_duplicate = $max_fid_after; + $max_fid_after = (int) \Drupal::entityQueryAggregate('file')->aggregate('fid', 'max')->execute()[0]['fid_max']; + $this->assertEqual($max_fid_before_duplicate, $max_fid_after, 'A new managed file was not created.'); + } + /** * Test extension handling. */ diff --git a/core/modules/jsonapi/tests/src/Functional/FileUploadTest.php b/core/modules/jsonapi/tests/src/Functional/FileUploadTest.php index d2b638cb2818b9819c830459851945c494742565..e57b8146999061e005d2ac52cbadc6106f186c89 100644 --- a/core/modules/jsonapi/tests/src/Functional/FileUploadTest.php +++ b/core/modules/jsonapi/tests/src/Functional/FileUploadTest.php @@ -440,6 +440,34 @@ public function testPostFileUploadDuplicateFile() { $this->assertSame($this->testFileData, file_get_contents('public://foobar/example_0.txt')); } + /** + * Tests using the file upload POST route twice, simulating a race condition. + * + * A validation error should occur when the filenames are not unique. + */ + public function testPostFileUploadDuplicateFileRaceCondition() { + $this->setUpAuthorization('POST'); + $this->config('jsonapi.settings')->set('read_only', FALSE)->save(TRUE); + + $uri = Url::fromUri('base:' . static::$postUri); + + // This request will have the default 'application/octet-stream' content + // type header. + $response = $this->fileRequest($uri, $this->testFileData); + + $this->assertSame(201, $response->getStatusCode()); + + // Simulate a race condition where two files are uploaded at almost the same + // time, by removing the first uploaded file from disk (leaving the entry in + // the file_managed table) before trying to upload another file with the + // same name. + unlink(\Drupal::service('file_system')->realpath('public://foobar/example.txt')); + + // Make the same request again. The upload should fail validation. + $response = $this->fileRequest($uri, $this->testFileData); + $this->assertResourceErrorResponse(422, PlainTextOutput::renderFromHtml("Unprocessable Entity: file validation failed.\nThe file public://foobar/example.txt already exists. Enter a unique file URI."), $uri, $response); + } + /** * Tests using the file upload route with any path prefixes being stripped. * diff --git a/core/modules/rest/tests/src/Functional/FileUploadResourceTestBase.php b/core/modules/rest/tests/src/Functional/FileUploadResourceTestBase.php index 994a4c3ccb51b079922c096e6e389fa33d9f1fea..a0926211038496e8149f3e4fe7387ed524da4589 100644 --- a/core/modules/rest/tests/src/Functional/FileUploadResourceTestBase.php +++ b/core/modules/rest/tests/src/Functional/FileUploadResourceTestBase.php @@ -310,6 +310,37 @@ public function testPostFileUploadDuplicateFile() { $this->assertSame($this->testFileData, file_get_contents('public://foobar/example_0.txt')); } + /** + * Tests using the file upload POST route twice, simulating a race condition. + * + * A validation error should occur when the filenames are not unique. + */ + public function testPostFileUploadDuplicateFileRaceCondition() { + $this->initAuthentication(); + + $this->provisionResource([static::$format], static::$auth ? [static::$auth] : [], ['POST']); + + $this->setUpAuthorization('POST'); + + $uri = Url::fromUri('base:' . static::$postUri); + + // This request will have the default 'application/octet-stream' content + // type header. + $response = $this->fileRequest($uri, $this->testFileData); + + $this->assertSame(201, $response->getStatusCode()); + + // Simulate a race condition where two files are uploaded at almost the same + // time, by removing the first uploaded file from disk (leaving the entry in + // the file_managed table) before trying to upload another file with the + // same name. + unlink(\Drupal::service('file_system')->realpath('public://foobar/example.txt')); + + // Make the same request again. The upload should fail validation. + $response = $this->fileRequest($uri, $this->testFileData); + $this->assertResourceErrorResponse(422, PlainTextOutput::renderFromHtml("Unprocessable Entity: validation failed.\nuri: The file public://foobar/example.txt already exists. Enter a unique file URI.\n"), $response); + } + /** * Tests using the file upload route with any path prefixes being stripped. *