.../Plugin/rest/resource/FileUploadResource.php | 5 -- .../File/FileUploadHalJsonTestBase.php | 59 +++++++++++----------- .../EventSubscriber/ResourceResponseSubscriber.php | 5 +- .../src/Functional/FileUploadResourceTestBase.php | 24 --------- 4 files changed, 33 insertions(+), 60 deletions(-) diff --git a/core/modules/file/src/Plugin/rest/resource/FileUploadResource.php b/core/modules/file/src/Plugin/rest/resource/FileUploadResource.php index 04b6712..a620419 100644 --- a/core/modules/file/src/Plugin/rest/resource/FileUploadResource.php +++ b/core/modules/file/src/Plugin/rest/resource/FileUploadResource.php @@ -173,7 +173,6 @@ public function permissions() { return []; } - /** * Creates a file from endpoint. * @@ -369,20 +368,17 @@ protected function validateAndParseContentDispositionHeader(Request $request) { */ protected function validateAndLoadFieldDefinition($entity_type_id, $bundle, $field_name) { $field_definitions = $this->entityFieldManager->getFieldDefinitions($entity_type_id, $bundle); - if (!isset($field_definitions[$field_name])) { throw new BadRequestHttpException(sprintf('Field "%s" does not exist', $field_name)); } /** @var \Drupal\Core\Field\FieldDefinitionInterface $field_definition */ $field_definition = $field_definitions[$field_name]; - if ($field_definition->getSetting('target_type') !== 'file') { throw new AccessDeniedHttpException(sprintf('"%s" is not a file field', $field_name)); } $entity_access_control_handler = $this->entityTypeManager->getAccessControlHandler($entity_type_id); - $access_result = $entity_access_control_handler->createAccess($bundle, NULL, [], TRUE) ->andIf($entity_access_control_handler->fieldAccess('edit', $field_definition, NULL, NULL, TRUE)); if (!($entity_access_control_handler->createAccess($bundle) && $entity_access_control_handler->fieldAccess('edit', $field_definition))) { @@ -436,7 +432,6 @@ protected function prepareFilename($filename, array &$validators) { // Build the list of non-munged extensions if the caller provided them. $extensions = $validators['file_validate_extensions'][0]; - if (!empty($extensions)) { // Munge the filename to protect against possible malicious extension // hiding within an unknown file type (ie: filename.html.foo). diff --git a/core/modules/hal/tests/src/Functional/EntityResource/File/FileUploadHalJsonTestBase.php b/core/modules/hal/tests/src/Functional/EntityResource/File/FileUploadHalJsonTestBase.php index cee894c..8f788ec 100644 --- a/core/modules/hal/tests/src/Functional/EntityResource/File/FileUploadHalJsonTestBase.php +++ b/core/modules/hal/tests/src/Functional/EntityResource/File/FileUploadHalJsonTestBase.php @@ -40,42 +40,41 @@ protected function getExpectedNormalizedEntity($fid = 1, $expected_filename = 'e unset($normalization['uid']); return $normalization + [ - '_links' => [ - 'self' => [ - // @todo This can use a proper link once - // https://www.drupal.org/project/drupal/issues/2907402 is complete. - // This link matches what is generated from from File::url(), a - // resource URL is currently not available. - 'href' => file_create_url($normalization['uri'][0]['value']), - ], - 'type' => [ - 'href' => $this->baseUrl . '/rest/type/file/file', - ], - $this->baseUrl . '/rest/relation/file/file/uid' => [ - ['href' => $this->baseUrl . '/user/' . $this->account->id() . '?_format=hal_json'] - ], + '_links' => [ + 'self' => [ + // @todo This can use a proper link once + // https://www.drupal.org/project/drupal/issues/2907402 is complete. + // This link matches what is generated from from File::url(), a + // resource URL is currently not available. + 'href' => file_create_url($normalization['uri'][0]['value']), + ], + 'type' => [ + 'href' => $this->baseUrl . '/rest/type/file/file', ], - '_embedded' => [ - $this->baseUrl . '/rest/relation/file/file/uid' => [ - [ - '_links' => [ - 'self' => [ - 'href' => $this->baseUrl . '/user/' . $this->account->id() . '?_format=hal_json', - ], - 'type' => [ - 'href' => $this->baseUrl . '/rest/type/user/user', - ], + $this->baseUrl . '/rest/relation/file/file/uid' => [ + ['href' => $this->baseUrl . '/user/' . $this->account->id() . '?_format=hal_json'] + ], + ], + '_embedded' => [ + $this->baseUrl . '/rest/relation/file/file/uid' => [ + [ + '_links' => [ + 'self' => [ + 'href' => $this->baseUrl . '/user/' . $this->account->id() . '?_format=hal_json', + ], + 'type' => [ + 'href' => $this->baseUrl . '/rest/type/user/user', ], - 'uuid' => [ - [ - 'value' => $this->account->uuid(), - ], + ], + 'uuid' => [ + [ + 'value' => $this->account->uuid(), ], ], ], ], - ]; + ], + ]; } - } diff --git a/core/modules/rest/src/EventSubscriber/ResourceResponseSubscriber.php b/core/modules/rest/src/EventSubscriber/ResourceResponseSubscriber.php index 9fbf385..babb238 100644 --- a/core/modules/rest/src/EventSubscriber/ResourceResponseSubscriber.php +++ b/core/modules/rest/src/EventSubscriber/ResourceResponseSubscriber.php @@ -89,7 +89,7 @@ public function onResponse(FilterResponseEvent $event) { * Determines the format to respond in. * * Respects the requested format if one is specified. However, it is common to - * forget to specify a request format in case of a POST or PATCH. Rather than + * forget to specify a response format in case of a POST or PATCH. Rather than * simply throwing an error, we apply the robustness principle: when POSTing * or PATCHing using a certain format, you probably expect a response in that * same format. @@ -119,6 +119,9 @@ public function getResponseFormat(RouteMatchInterface $route_match, Request $req return $requested_format; } + // If a request body is present, then use the format corresponding to the + // request body's Content-Type for the response, if it's an acceptable + // format for the request. if (!empty($request->getContent()) && in_array($content_type_format, $acceptable_request_formats, TRUE)) { return $content_type_format; } diff --git a/core/modules/rest/tests/src/Functional/FileUploadResourceTestBase.php b/core/modules/rest/tests/src/Functional/FileUploadResourceTestBase.php index 88a9510..7e1d189 100644 --- a/core/modules/rest/tests/src/Functional/FileUploadResourceTestBase.php +++ b/core/modules/rest/tests/src/Functional/FileUploadResourceTestBase.php @@ -145,11 +145,8 @@ public function testPostFileUpload() { // This request will have the default 'application/octet-stream' content // type header. $response = $this->fileRequest($uri, $this->testFileData); - $this->assertSame(201, $response->getStatusCode()); - $expected = $this->getExpectedNormalizedEntity(); - $this->assertResponseData($expected, $response); // Check the actual file data. @@ -158,11 +155,8 @@ public function testPostFileUpload() { // Test the file again but using 'filename' in the Content-Disposition // header with no 'file' prefix. $response = $this->fileRequest($uri, $this->testFileData, ['Content-Disposition' => 'filename="example.txt"']); - $this->assertSame(201, $response->getStatusCode()); - $expected = $this->getExpectedNormalizedEntity(2, 'example_0.txt'); - $this->assertResponseData($expected, $response); // Check the actual file data. @@ -232,12 +226,10 @@ public function testPostFileUploadDuplicateFile() { // Make the same request again. The file should be saved as a new file // entity that has the same file name but a suffixed file URI. $response = $this->fileRequest($uri, $this->testFileData); - $this->assertSame(201, $response->getStatusCode()); // Loading expected normalized data for file 2, the duplicate file. $expected = $this->getExpectedNormalizedEntity(2, 'example_0.txt'); - $this->assertResponseData($expected, $response); // Check the actual file data. @@ -257,9 +249,7 @@ public function testFileUploadStrippedFilePath() { $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); @@ -281,9 +271,7 @@ public function testFileUploadUnicodeFilename() { $uri = Url::fromUri('base:' . static::$postUri); $response = $this->fileRequest($uri, $this->testFileData, ['Content-Disposition' => 'file; filename="example-✓.txt"']); - $this->assertSame(201, $response->getStatusCode()); - $expected = $this->getExpectedNormalizedEntity(1, 'example-✓.txt', TRUE); $this->assertResponseData($expected, $response); @@ -306,13 +294,10 @@ public function testFileUploadZeroByteFile() { // Test with a zero byte file. $response = $this->fileRequest($uri, NULL); - $this->assertSame(201, $response->getStatusCode()); - $expected = $this->getExpectedNormalizedEntity(); // Modify the default expected data to account for the 0 byte file. $expected['filesize'][0]['value'] = 0; - $this->assertResponseData($expected, $response); // Check the actual file data. @@ -333,7 +318,6 @@ public function testFileUploadInvalidFileType() { // Test with a JSON file. $response = $this->fileRequest($uri, '{"test":123}', ['Content-Disposition' => 'filename="example.json"']); - $this->assertResourceErrorResponse(422, PlainTextOutput::renderFromHtml("Unprocessable Entity: file validation failed.\nOnly files with the following extensions are allowed: txt."), $response); // Make sure that no file was saved. @@ -360,7 +344,6 @@ public function testFileUploadLargerFileSize() { // Generate a string larger than the 50 byte limit set. $response = $this->fileRequest($uri, $this->randomString(100)); - $this->assertResourceErrorResponse(422, PlainTextOutput::renderFromHtml("Unprocessable Entity: file validation failed.\nThe file is 100 bytes exceeding the maximum file size of 50 bytes."), $response); // Make sure that no file was saved. @@ -384,13 +367,10 @@ public function testFileUploadMaliciousExtension() { // Test using a masked exploit file. $response = $this->fileRequest($uri, $php_string, ['Content-Disposition' => 'filename="example.php.txt"']); - $expected = $this->getExpectedNormalizedEntity(1, 'example.php_.txt', TRUE); // Override the expected filesize. $expected['filesize'][0]['value'] = strlen($php_string); - $this->assertResponseData($expected, $response); - $this->assertTrue(file_exists('public://foobar/example.php_.txt')); // Add php as an allowed format. Allow insecure uploads still being FALSE @@ -401,11 +381,9 @@ public function testFileUploadMaliciousExtension() { $this->refreshTestStateAfterRestConfigChange(); $response = $this->fileRequest($uri, $php_string, ['Content-Disposition' => 'filename="example.php"']); - $expected = $this->getExpectedNormalizedEntity(2, 'example.php.txt', TRUE); // Override the expected filesize. $expected['filesize'][0]['value'] = strlen($php_string); - $this->assertResponseData($expected, $response); // Now allow insecure uploads. @@ -415,13 +393,11 @@ public function testFileUploadMaliciousExtension() { ->save(); $response = $this->fileRequest($uri, $php_string, ['Content-Disposition' => 'filename="example.php"']); - $expected = $this->getExpectedNormalizedEntity(3, 'example.php', TRUE); // Override the expected filesize. $expected['filesize'][0]['value'] = strlen($php_string); // The file mime should also now be PHP. $expected['filemime'][0]['value'] = 'application/x-httpd-php'; - $this->assertResponseData($expected, $response); }