diff --git a/core/modules/rest/src/EventSubscriber/ResourceResponseSubscriber.php b/core/modules/rest/src/EventSubscriber/ResourceResponseSubscriber.php index df77281f37f..ef33817ce85 100644 --- a/core/modules/rest/src/EventSubscriber/ResourceResponseSubscriber.php +++ b/core/modules/rest/src/EventSubscriber/ResourceResponseSubscriber.php @@ -79,7 +79,7 @@ class ResourceResponseSubscriber implements EventSubscriberInterface { * 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. @@ -94,33 +94,35 @@ class ResourceResponseSubscriber implements EventSubscriberInterface { */ public function getResponseFormat(RouteMatchInterface $route_match, Request $request) { $route = $route_match->getRouteObject(); - $acceptable_request_formats = $route->hasRequirement('_format') ? explode('|', $route->getRequirement('_format')) : []; - $acceptable_content_type_formats = $route->hasRequirement('_content_type_format') ? explode('|', $route->getRequirement('_content_type_format')) : []; - $acceptable_formats = $request->isMethodCacheable() ? $acceptable_request_formats : $acceptable_content_type_formats; + $acceptable_response_formats = $route->hasRequirement('_format') ? explode('|', $route->getRequirement('_format')) : []; + $acceptable_request_formats = $route->hasRequirement('_content_type_format') ? explode('|', $route->getRequirement('_content_type_format')) : []; + $acceptable_formats = $request->isMethodCacheable() ? $acceptable_response_formats : $acceptable_request_formats; $requested_format = $request->getRequestFormat(); $content_type_format = $request->getContentType(); - // If an acceptable format is requested, then use that. Otherwise, including - // and particularly when the client forgot to specify a format, then use - // heuristics to select the format that is most likely expected. - if (in_array($requested_format, $acceptable_formats)) { + // If an acceptable response format is requested, then use that. Otherwise, + // including and particularly when the client forgot to specify a response + // format, then use heuristics to select the format that is most likely + // expected. + if (in_array($requested_format, $acceptable_response_formats, TRUE)) { 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. - elseif (!empty($request->getContent()) && in_array($content_type_format, $acceptable_content_type_formats)) { + if (!empty($request->getContent()) && in_array($content_type_format, $acceptable_request_formats, TRUE)) { return $content_type_format; } + // Otherwise, use the first acceptable format. - elseif (!empty($acceptable_formats)) { + if (!empty($acceptable_formats)) { return $acceptable_formats[0]; } + // Sometimes, there are no acceptable formats, e.g. DELETE routes. - else { - return NULL; - } + return NULL; } /** diff --git a/core/modules/rest/tests/src/Unit/EventSubscriber/ResourceResponseSubscriberTest.php b/core/modules/rest/tests/src/Unit/EventSubscriber/ResourceResponseSubscriberTest.php index d09fdced4cd..eab2d9f89ba 100644 --- a/core/modules/rest/tests/src/Unit/EventSubscriber/ResourceResponseSubscriberTest.php +++ b/core/modules/rest/tests/src/Unit/EventSubscriber/ResourceResponseSubscriberTest.php @@ -78,7 +78,7 @@ class ResourceResponseSubscriberTest extends UnitTestCase { * * @dataProvider providerTestResponseFormat */ - public function testResponseFormat($methods, array $supported_formats, $request_format, array $request_headers, $request_body, $expected_response_format, $expected_response_content_type, $expected_response_content) { + public function testResponseFormat($methods, array $supported_response_formats, array $supported_request_formats, $request_format, array $request_headers, $request_body, $expected_response_format, $expected_response_content_type, $expected_response_content) { foreach ($request_headers as $key => $value) { unset($request_headers[$key]); $key = strtoupper(str_replace('-', '_', $key)); @@ -92,8 +92,10 @@ class ResourceResponseSubscriberTest extends UnitTestCase { if ($request_format) { $request->setRequestFormat($request_format); } - $route_requirement_key_format = $request->isMethodCacheable() ? '_format' : '_content_type_format'; - $route_match = new RouteMatch('test', new Route('/rest/test', ['_rest_resource_config' => $this->randomMachineName()], [$route_requirement_key_format => implode('|', $supported_formats)])); + + $route_requirements = $this->generateRouteRequirements($supported_response_formats, $supported_request_formats); + + $route_match = new RouteMatch('test', new Route('/rest/test', ['_rest_resource_config' => $this->randomMachineName()], $route_requirements)); $resource_response_subscriber = new ResourceResponseSubscriber( $this->prophesize(SerializerInterface::class)->reveal(), @@ -113,9 +115,7 @@ class ResourceResponseSubscriberTest extends UnitTestCase { * * @dataProvider providerTestResponseFormat */ - public function testOnResponseWithCacheableResponse($methods, array $supported_formats, $request_format, array $request_headers, $request_body, $expected_response_format, $expected_response_content_type, $expected_response_content) { - $rest_config_name = $this->randomMachineName(); - + public function testOnResponseWithCacheableResponse($methods, array $supported_response_formats, array $supported_request_formats, $request_format, array $request_headers, $request_body, $expected_response_format, $expected_response_content_type, $expected_response_content) { foreach ($request_headers as $key => $value) { unset($request_headers[$key]); $key = strtoupper(str_replace('-', '_', $key)); @@ -129,8 +129,10 @@ class ResourceResponseSubscriberTest extends UnitTestCase { if ($request_format) { $request->setRequestFormat($request_format); } - $route_requirement_key_format = $request->isMethodCacheable() ? '_format' : '_content_type_format'; - $route_match = new RouteMatch('test', new Route('/rest/test', ['_rest_resource_config' => $rest_config_name], [$route_requirement_key_format => implode('|', $supported_formats)])); + + $route_requirements = $this->generateRouteRequirements($supported_response_formats, $supported_request_formats); + + $route_match = new RouteMatch('test', new Route('/rest/test', ['_rest_resource_config' => $this->randomMachineName()], $route_requirements)); // The RequestHandler must return a ResourceResponseInterface object. $handler_response = new ResourceResponse($method !== 'DELETE' ? ['REST' => 'Drupal'] : NULL); @@ -163,9 +165,7 @@ class ResourceResponseSubscriberTest extends UnitTestCase { * * @dataProvider providerTestResponseFormat */ - public function testOnResponseWithUncacheableResponse($methods, array $supported_formats, $request_format, array $request_headers, $request_body, $expected_response_format, $expected_response_content_type, $expected_response_content) { - $rest_config_name = $this->randomMachineName(); - + public function testOnResponseWithUncacheableResponse($methods, array $supported_response_formats, array $supported_request_formats, $request_format, array $request_headers, $request_body, $expected_response_format, $expected_response_content_type, $expected_response_content) { foreach ($request_headers as $key => $value) { unset($request_headers[$key]); $key = strtoupper(str_replace('-', '_', $key)); @@ -179,8 +179,10 @@ class ResourceResponseSubscriberTest extends UnitTestCase { if ($request_format) { $request->setRequestFormat($request_format); } - $route_requirement_key_format = $request->isMethodCacheable() ? '_format' : '_content_type_format'; - $route_match = new RouteMatch('test', new Route('/rest/test', ['_rest_resource_config' => $rest_config_name], [$route_requirement_key_format => implode('|', $supported_formats)])); + + $route_requirements = $this->generateRouteRequirements($supported_response_formats, $supported_request_formats); + + $route_match = new RouteMatch('test', new Route('/rest/test', ['_rest_resource_config' => $this->randomMachineName()], $route_requirements)); // The RequestHandler must return a ResourceResponseInterface object. $handler_response = new ModifiedResourceResponse($method !== 'DELETE' ? ['REST' => 'Drupal'] : NULL); @@ -225,6 +227,7 @@ class ResourceResponseSubscriberTest extends UnitTestCase { // @todo add 'HEAD' in https://www.drupal.org/node/2752325 ['GET'], ['xml', 'json'], + [], 'json', [], NULL, @@ -236,6 +239,7 @@ class ResourceResponseSubscriberTest extends UnitTestCase { // @todo add 'HEAD' in https://www.drupal.org/node/2752325 ['GET'], ['xml', 'json'], + [], 'xml', [], NULL, @@ -247,6 +251,7 @@ class ResourceResponseSubscriberTest extends UnitTestCase { // @todo add 'HEAD' in https://www.drupal.org/node/2752325 ['GET'], ['json', 'xml'], + [], FALSE, [], NULL, @@ -258,6 +263,7 @@ class ResourceResponseSubscriberTest extends UnitTestCase { // @todo add 'HEAD' in https://www.drupal.org/node/2752325 ['GET'], ['xml', 'json'], + [], FALSE, [], NULL, @@ -271,6 +277,7 @@ class ResourceResponseSubscriberTest extends UnitTestCase { 'unsafe methods with response (POST, PATCH): client requested no format, response should use request body format (JSON)' => [ ['POST', 'PATCH'], ['xml', 'json'], + ['xml', 'json'], FALSE, ['Content-Type' => 'application/json'], $json_encoded, @@ -281,6 +288,7 @@ class ResourceResponseSubscriberTest extends UnitTestCase { 'unsafe methods with response (POST, PATCH): client requested no format, response should use request body format (XML)' => [ ['POST', 'PATCH'], ['xml', 'json'], + ['xml', 'json'], FALSE, ['Content-Type' => 'text/xml'], $xml_encoded, @@ -291,6 +299,7 @@ class ResourceResponseSubscriberTest extends UnitTestCase { 'unsafe methods with response (POST, PATCH): client requested format other than request body format (JSON): response format should use requested format (XML)' => [ ['POST', 'PATCH'], ['xml', 'json'], + ['xml', 'json'], 'xml', ['Content-Type' => 'application/json'], $json_encoded, @@ -301,6 +310,7 @@ class ResourceResponseSubscriberTest extends UnitTestCase { 'unsafe methods with response (POST, PATCH): client requested format other than request body format (XML), but is allowed for the request body (JSON)' => [ ['POST', 'PATCH'], ['xml', 'json'], + ['xml', 'json'], 'json', ['Content-Type' => 'text/xml'], $xml_encoded, @@ -308,12 +318,35 @@ class ResourceResponseSubscriberTest extends UnitTestCase { 'application/json', $json_encoded, ], + 'unsafe methods with response (POST, PATCH): client requested format other than request body format when only XML is allowed as a content type format' => [ + ['POST', 'PATCH'], + ['xml'], + ['json'], + 'json', + ['Content-Type' => 'text/xml'], + $xml_encoded, + 'json', + 'application/json', + $json_encoded, + ], + 'unsafe methods with response (POST, PATCH): client requested format other than request body format when only JSON is allowed as a content type format' => [ + ['POST', 'PATCH'], + ['json'], + ['xml'], + 'xml', + ['Content-Type' => 'application/json'], + $json_encoded, + 'xml', + 'text/xml', + $xml_encoded, + ], ]; $unsafe_method_bodyless_test_cases = [ - 'unsafe methods with response bodies (DELETE): client requested no format, response should have no format' => [ + 'unsafe methods without response bodies (DELETE): client requested no format, response should have no format' => [ ['DELETE'], ['xml', 'json'], + ['xml', 'json'], FALSE, ['Content-Type' => 'application/json'], NULL, @@ -321,9 +354,10 @@ class ResourceResponseSubscriberTest extends UnitTestCase { NULL, '', ], - 'unsafe methods with response bodies (DELETE): client requested format (XML), response should have no format' => [ + 'unsafe methods without response bodies (DELETE): client requested format (XML), response should have no format' => [ ['DELETE'], ['xml', 'json'], + ['xml', 'json'], 'xml', ['Content-Type' => 'application/json'], NULL, @@ -331,9 +365,10 @@ class ResourceResponseSubscriberTest extends UnitTestCase { NULL, '', ], - 'unsafe methods with response bodies (DELETE): client requested format (JSON), response should have no format' => [ + 'unsafe methods without response bodies (DELETE): client requested format (JSON), response should have no format' => [ ['DELETE'], ['xml', 'json'], + ['xml', 'json'], 'json', ['Content-Type' => 'application/json'], NULL, @@ -368,4 +403,26 @@ class ResourceResponseSubscriberTest extends UnitTestCase { return $resource_response_subscriber; } + /** + * Generates route requirements based on supported formats. + * + * @param array $supported_response_formats + * The supported response formats to add to the route requirements. + * @param array $supported_request_formats + * The supported request formats to add to the route requirements. + * + * @return array + * An array of route requirements. + */ + protected function generateRouteRequirements(array $supported_response_formats, array $supported_request_formats) { + $route_requirements = [ + '_format' => implode('|', $supported_response_formats), + ]; + if (!empty($supported_request_formats)) { + $route_requirements['_content_type_format'] = implode('|', $supported_request_formats); + } + + return $route_requirements; + } + }