.../Core/StackMiddleware/NegotiationMiddleware.php | 8 ++-- .../EntityResource/EntityResourceTestBase.php | 53 ++++++++++++++-------- .../default_format_test.info.yml | 6 +++ .../default_format_test.routing.yml | 17 +++++++ .../src/DefaultFormatTestController.php | 15 ++++++ .../FunctionalTests/Routing/DefaultFormatTest.php | 27 +++++++++++ .../StackMiddleware/NegotiationMiddlewareTest.php | 10 ++-- 7 files changed, 110 insertions(+), 26 deletions(-) diff --git a/core/lib/Drupal/Core/StackMiddleware/NegotiationMiddleware.php b/core/lib/Drupal/Core/StackMiddleware/NegotiationMiddleware.php index e386461..f246cce 100644 --- a/core/lib/Drupal/Core/StackMiddleware/NegotiationMiddleware.php +++ b/core/lib/Drupal/Core/StackMiddleware/NegotiationMiddleware.php @@ -46,7 +46,9 @@ public function handle(Request $request, $type = self::MASTER_REQUEST, $catch = } // Determine the request format using the negotiator. - $request->setRequestFormat($this->getContentType($request)); + if ($requested_format = $this->getContentType($request)) { + $request->setRequestFormat($requested_format); + } return $this->app->handle($request, $type, $catch); } @@ -88,8 +90,8 @@ protected function getContentType(Request $request) { return $request->query->get('_format'); } - // Do HTML last so that it always wins. - return 'html'; + // No format was specified in the request. + return NULL; } } diff --git a/core/modules/rest/tests/src/Functional/EntityResource/EntityResourceTestBase.php b/core/modules/rest/tests/src/Functional/EntityResource/EntityResourceTestBase.php index d631463..3f5bcf4 100644 --- a/core/modules/rest/tests/src/Functional/EntityResource/EntityResourceTestBase.php +++ b/core/modules/rest/tests/src/Functional/EntityResource/EntityResourceTestBase.php @@ -156,12 +156,19 @@ /** * Provides an entity resource. + * + * @param bool $single_format + * Provisions a single-format entity REST resource. Defaults to FALSE. */ - protected function provisionEntityResource() { + protected function provisionEntityResource($single_format = FALSE) { + $this->resourceConfigStorage->delete($this->resourceConfigStorage->loadMultiple()); + $format = $single_format + ? [static::$format] + : [static::$format, 'xml']; // It's possible to not have any authentication providers enabled, when // testing public (anonymous) usage of a REST resource. $auth = isset(static::$auth) ? [static::$auth] : []; - $this->provisionResource([static::$format], $auth); + $this->provisionResource($format, $auth); } /** @@ -434,20 +441,6 @@ public function testGet() { } $this->provisionEntityResource(); - // Simulate the developer again forgetting the ?_format query string. - $url->setOption('query', []); - - // DX: 406 when ?_format is missing, except when requesting a canonical HTML - // route. - $response = $this->request('GET', $url, $request_options); - if ($has_canonical_url && (!static::$auth || static::$auth === 'cookie')) { - $this->assertSame(403, $response->getStatusCode()); - } - else { - $this->assert406Response($response); - } - - $url->setOption('query', ['_format' => static::$format]); // DX: forgetting authentication: authentication provider-specific error // response. @@ -472,10 +465,34 @@ public function testGet() { unset($request_options[RequestOptions::HEADERS]['REST-test-auth-global']); $request_options = NestedArray::mergeDeep($request_options, $this->getAuthenticationRequestOptions('GET')); - // DX: 403 when unauthorized. + // First: single format. Drupal will automatically pick the only format. + $this->provisionEntityResource(TRUE); + // DX: 403 because unauthorized single-format route, ?_format is omittable. + $url->setOption('query', []); + $response = $this->request('GET', $url, $request_options); + $this->assertSame(403, $response->getStatusCode()); + $this->assertSame(['text/html; charset=UTF-8'], $response->getHeader('Content-Type')); + $this->assertSame(static::$auth ? [] : ['MISS'], $response->getHeader('X-Drupal-Cache')); + $this->assertSame(['MISS'], $response->getHeader('X-Drupal-Dynamic-Cache')); + // DX: 403 because unauthorized. + $url->setOption('query', ['_format' => static::$format]); $response = $this->request('GET', $url, $request_options); $expected_403_cacheability = $this->getExpectedUnauthorizedAccessCacheability(); - $this->assertResourceErrorResponse(403, $this->getExpectedUnauthorizedAccessMessage('GET'), $response, $expected_403_cacheability->getCacheTags(), $expected_403_cacheability->getCacheContexts(), static::$auth ? FALSE : 'MISS', 'MISS'); + $this->assertResourceErrorResponse(403, FALSE, $response, $expected_403_cacheability->getCacheTags(), $expected_403_cacheability->getCacheContexts(), static::$auth ? FALSE : 'MISS', 'MISS'); + + // Then, what we'll use for the remainder of the test: multiple formats. + $this->provisionEntityResource(); + // DX: 406 because despite unauthorized, ?_format is not omittable. + $url->setOption('query', []); + $response = $this->request('GET', $url, $request_options); + $this->assertSame(406, $response->getStatusCode()); + $this->assertSame(['text/html; charset=UTF-8'], $response->getHeader('Content-Type')); + $this->assertSame(static::$auth ? [] : ['MISS'], $response->getHeader('X-Drupal-Cache')); + $this->assertSame(['UNCACHEABLE'], $response->getHeader('X-Drupal-Dynamic-Cache')); + // DX: 403 because unauthorized. + $url->setOption('query', ['_format' => static::$format]); + $response = $this->request('GET', $url, $request_options); + $this->assertResourceErrorResponse(403, $this->getExpectedUnauthorizedAccessMessage('GET'), $response, $expected_403_cacheability->getCacheTags(), $expected_403_cacheability->getCacheContexts(), static::$auth ? FALSE : 'MISS', 'HIT'); $this->assertArrayNotHasKey('Link', $response->getHeaders()); $this->setUpAuthorization('GET'); diff --git a/core/modules/system/tests/modules/default_format_test/default_format_test.info.yml b/core/modules/system/tests/modules/default_format_test/default_format_test.info.yml new file mode 100644 index 0000000..0a02a06 --- /dev/null +++ b/core/modules/system/tests/modules/default_format_test/default_format_test.info.yml @@ -0,0 +1,6 @@ +name: 'Default format test' +type: module +description: 'Support module for testing default route format.' +package: Testing +version: VERSION +core: 8.x diff --git a/core/modules/system/tests/modules/default_format_test/default_format_test.routing.yml b/core/modules/system/tests/modules/default_format_test/default_format_test.routing.yml new file mode 100644 index 0000000..9add41b --- /dev/null +++ b/core/modules/system/tests/modules/default_format_test/default_format_test.routing.yml @@ -0,0 +1,17 @@ +default_format_test.machine: + path: '/default_format_test/machine' + defaults: + # Same controller + method! + _controller: '\Drupal\default_format_test\DefaultFormatTestController::content' + requirements: + _access: 'TRUE' + _format: 'json' + +default_format_test.human: + path: '/default_format_test/human' + defaults: + # Same controller + method! + _controller: '\Drupal\default_format_test\DefaultFormatTestController::content' + requirements: + _access: 'TRUE' + _format: 'html' \ No newline at end of file diff --git a/core/modules/system/tests/modules/default_format_test/src/DefaultFormatTestController.php b/core/modules/system/tests/modules/default_format_test/src/DefaultFormatTestController.php new file mode 100644 index 0000000..af90187 --- /dev/null +++ b/core/modules/system/tests/modules/default_format_test/src/DefaultFormatTestController.php @@ -0,0 +1,15 @@ +getRequestFormat(); + return new CacheableResponse('format:' . $format, 200, ['Content-Type' => $request->getMimeType($format)]); + } + +} \ No newline at end of file diff --git a/core/tests/Drupal/FunctionalTests/Routing/DefaultFormatTest.php b/core/tests/Drupal/FunctionalTests/Routing/DefaultFormatTest.php new file mode 100644 index 0000000..fba590e --- /dev/null +++ b/core/tests/Drupal/FunctionalTests/Routing/DefaultFormatTest.php @@ -0,0 +1,27 @@ +drupalGet('/default_format_test/human'); + $this->assertSame('format:html', $this->getSession()->getPage()->getContent()); + $this->assertSame('MISS', $this->drupalGetHeader('X-Drupal-Cache')); + + $this->drupalGet('/default_format_test/machine'); + $this->assertSame('format:html', $this->getSession()->getPage()->getContent()); + $this->assertSame('MISS', $this->drupalGetHeader('X-Drupal-Cache')); + } + +} diff --git a/core/tests/Drupal/Tests/Core/StackMiddleware/NegotiationMiddlewareTest.php b/core/tests/Drupal/Tests/Core/StackMiddleware/NegotiationMiddlewareTest.php index 7b3a0d4..fad9a76 100644 --- a/core/tests/Drupal/Tests/Core/StackMiddleware/NegotiationMiddlewareTest.php +++ b/core/tests/Drupal/Tests/Core/StackMiddleware/NegotiationMiddlewareTest.php @@ -68,10 +68,10 @@ public function testFormatViaQueryParameter() { * * @covers ::getContentType */ - public function testUnknowContentTypeReturnsHtmlByDefault() { + public function testUnknowContentTypeReturnsNull() { $request = new Request(); - $this->assertSame('html', $this->contentNegotiation->getContentType($request)); + $this->assertNull($this->contentNegotiation->getContentType($request)); } /** @@ -83,7 +83,7 @@ public function testUnknowContentTypeButAjaxRequest() { $request = new Request(); $request->headers->set('X-Requested-With', 'XMLHttpRequest'); - $this->assertSame('html', $this->contentNegotiation->getContentType($request)); + $this->assertNull($this->contentNegotiation->getContentType($request)); } /** @@ -98,7 +98,7 @@ public function testHandle() { $request->setFormat()->shouldNotBeCalled(); // Request format will be set with default format. - $request->setRequestFormat('html')->shouldBeCalled(); + $request->setRequestFormat()->shouldNotBeCalled(); // Some getContentType calls we don't really care about but have to mock. $request_data = $this->prophesize(ParameterBag::class); @@ -127,7 +127,7 @@ public function testSetFormat() { $request->setFormat('david', 'geeky/david')->shouldBeCalled(); // Some calls we don't care about. - $request->setRequestFormat('html')->shouldBeCalled(); + $request->setRequestFormat()->shouldNotBeCalled(); $request_data = $this->prophesize(ParameterBag::class); $request_data->get('ajax_iframe_upload', FALSE)->shouldBeCalled(); $request_mock = $request->reveal();