diff --git a/core/lib/Drupal/Core/Routing/AcceptHeaderMatcher.php b/core/lib/Drupal/Core/Routing/AcceptHeaderMatcher.php index 7ebc4d3..29b111f 100644 --- a/core/lib/Drupal/Core/Routing/AcceptHeaderMatcher.php +++ b/core/lib/Drupal/Core/Routing/AcceptHeaderMatcher.php @@ -42,46 +42,42 @@ public function filter(RouteCollection $collection, Request $request) { // Generates a list of Symfony formats matching the acceptable MIME types. // @todo replace by proper content negotiation library. $acceptable_mime_types = $request->getAcceptableContentTypes(); - $acceptable_formats = array_map(array($request, 'getFormat'), $acceptable_mime_types); + $acceptable_formats = array_filter(array_map(array($request, 'getFormat'), $acceptable_mime_types)); $primary_format = $this->contentNegotiation->getContentType($request); // Collect a list of routes that match the primary request content type. $primary_matches = new RouteCollection(); // List of routes that match any of multiple specified content types in the - // request. + // request, which should get a lower priority. $somehow_matches = new RouteCollection(); foreach ($collection as $name => $route) { // _format could be a |-delimited list of supported formats. $supported_formats = array_filter(explode('|', $route->getRequirement('_format'))); - if (in_array($primary_format, $supported_formats)) { - $primary_matches->add($name, $route); - continue; - } - - // HTML is the default format if the route does not specify it. We also - // add the other Drupal AJAX and JSON formats here to cover general use - // cases. if (empty($supported_formats)) { - $supported_formats = array('html', 'drupal_ajax', 'drupal_modal', 'drupal_dialog', 'json'); + // No format restriction on the route, so it always matches. + $somehow_matches->add($name, $route); + } + elseif (in_array($primary_format, $supported_formats)) { + // Perfect match, which will get a higher priority. + $primary_matches->add($name, $route); } // The route partially matches if it doesn't care about format, if it // explicitly allows any format, or if one of its allowed formats is // in the request's list of acceptable formats. - if (in_array('*/*', $acceptable_mime_types) || array_intersect($acceptable_formats, $supported_formats)) { + elseif (in_array('*/*', $acceptable_mime_types) || array_intersect($acceptable_formats, $supported_formats)) { $somehow_matches->add($name, $route); } } + // Append the generic routes to the end, which will give them a lower + // priority. + $primary_matches->addCollection($somehow_matches); if (count($primary_matches)) { return $primary_matches; } - if (count($somehow_matches)) { - return $somehow_matches; - } - // We do not throw a // \Symfony\Component\Routing\Exception\ResourceNotFoundException here // because we don't want to return a 404 status code, but rather a 406. diff --git a/core/lib/Drupal/Core/Routing/ContentTypeHeaderMatcher.php b/core/lib/Drupal/Core/Routing/ContentTypeHeaderMatcher.php index 04bd40b..3b32bf9 100644 --- a/core/lib/Drupal/Core/Routing/ContentTypeHeaderMatcher.php +++ b/core/lib/Drupal/Core/Routing/ContentTypeHeaderMatcher.php @@ -29,20 +29,16 @@ public function filter(RouteCollection $collection, Request $request) { } $format = $request->getContentType(); - if ($format === NULL) { - // Even if the request has no Content-type header we initialize it here - // with a default so that we can filter out routes that require a - // different one later. - $format = 'html'; - } + foreach ($collection as $name => $route) { $supported_formats = array_filter(explode('|', $route->getRequirement('_content_type_format'))); if (empty($supported_formats)) { - // The route has not specified any Content-Type restrictions, so we - // assume default restrictions. - $supported_formats = array('html', 'drupal_ajax', 'drupal_modal', 'drupal_dialog'); + // No restriction on the route, so we move the route to the end of the + // collection by re-adding it. That way generic routes sink down in the + // list and exact matching routes stay on top. + $collection->add($name, $route); } - if (!in_array($format, $supported_formats)) { + elseif (!in_array($format, $supported_formats)) { $collection->remove($name); } } diff --git a/core/modules/rest/lib/Drupal/rest/Tests/NodeTest.php b/core/modules/rest/lib/Drupal/rest/Tests/NodeTest.php index 56b7188..65a7f1d 100644 --- a/core/modules/rest/lib/Drupal/rest/Tests/NodeTest.php +++ b/core/modules/rest/lib/Drupal/rest/Tests/NodeTest.php @@ -57,6 +57,13 @@ public function testNodes() { $node->save(); $this->httpRequest('node/' . $node->id(), 'GET', NULL, $this->defaultMimeType); $this->assertResponse(200); + $this->assertHeader('Content-type', $this->defaultMimeType); + + // Also check that JSON works and the routing system selects the correct + // REST route. + $this->httpRequest('node/' . $node->id(), 'GET', NULL, 'application/json'); + $this->assertResponse(200); + $this->assertHeader('Content-type', 'application/json'); // Check that a simple PATCH update to the node title works as expected. $this->enableNodeConfiguration('PATCH', 'update'); diff --git a/core/modules/rest/lib/Drupal/rest/Tests/ReadTest.php b/core/modules/rest/lib/Drupal/rest/Tests/ReadTest.php index 3be1b33..071a598 100644 --- a/core/modules/rest/lib/Drupal/rest/Tests/ReadTest.php +++ b/core/modules/rest/lib/Drupal/rest/Tests/ReadTest.php @@ -60,8 +60,16 @@ public function testRead() { // Try to read the entity with an unsupported mime format. $response = $this->httpRequest($this->entityBasePath($entity_type) . '/' . $entity->id(), 'GET', NULL, 'application/wrongformat'); - $this->assertResponse(406); - $this->assertEqual($response, 'An error occurred: No route found for the specified formats application/wrongformat.'); + if ($entity_type == 'entity_test') { + $this->assertResponse(406); + $this->assertEqual($response, 'An error occurred: No route found for the specified formats application/wrongformat.'); + } + if ($entity_type == 'node') { + // Nodes are special because there are HTML routes without format + // restrictions, so we are hitting the standard node view route here. + $this->assertResponse(200); + $this->assertHeader('Content-type', 'text/html; charset=UTF-8'); + } // Try to read an entity that does not exist. $response = $this->httpRequest($this->entityBasePath($entity_type) . '/9999', 'GET', NULL, $this->defaultMimeType); diff --git a/core/modules/system/lib/Drupal/system/Tests/Routing/AcceptHeaderMatcherTest.php b/core/modules/system/lib/Drupal/system/Tests/Routing/AcceptHeaderMatcherTest.php index 99e7901..34b38a1 100644 --- a/core/modules/system/lib/Drupal/system/Tests/Routing/AcceptHeaderMatcherTest.php +++ b/core/modules/system/lib/Drupal/system/Tests/Routing/AcceptHeaderMatcherTest.php @@ -7,9 +7,9 @@ namespace Drupal\system\Tests\Routing; +use Drupal\Core\ContentNegotiation; use Drupal\Core\Routing\AcceptHeaderMatcher; use Drupal\simpletest\UnitTestBase; - use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpKernel\Exception\NotAcceptableHttpException; @@ -44,7 +44,7 @@ function __construct($test_id = NULL) { */ public function testFilterRoutes() { - $matcher = new AcceptHeaderMatcher(); + $matcher = new AcceptHeaderMatcher(new ContentNegotiation()); $collection = $this->fixtures->sampleRouteCollection(); // Tests basic JSON request. @@ -54,6 +54,10 @@ public function testFilterRoutes() { $this->assertEqual(count($routes), 4, 'The correct number of routes was found.'); $this->assertNotNull($routes->get('route_c'), 'The json route was found.'); $this->assertNull($routes->get('route_e'), 'The html route was not found.'); + foreach ($routes as $name => $route) { + $this->assertEqual($name, 'route_c', 'The json route is the first one in the collection.'); + break; + } // Tests JSON request with alternative JSON MIME type Accept header. $request = Request::create('path/two', 'GET'); @@ -62,6 +66,10 @@ public function testFilterRoutes() { $this->assertEqual(count($routes), 4, 'The correct number of routes was found.'); $this->assertNotNull($routes->get('route_c'), 'The json route was found.'); $this->assertNull($routes->get('route_e'), 'The html route was not found.'); + foreach ($routes as $name => $route) { + $this->assertEqual($name, 'route_c', 'The json route is the first one in the collection.'); + break; + } // Tests basic HTML request. $request = Request::create('path/two', 'GET'); @@ -70,13 +78,17 @@ public function testFilterRoutes() { $this->assertEqual(count($routes), 4, 'The correct number of routes was found.'); $this->assertNull($routes->get('route_c'), 'The json route was not found.'); $this->assertNotNull($routes->get('route_e'), 'The html route was found.'); + foreach ($routes as $name => $route) { + $this->assertEqual($name, 'route_e', 'The html route is the first one in the collection.'); + break; + } } /** * Confirms that the AcceptHeaderMatcher matcher throws an exception for no-route. */ public function testNoRouteFound() { - $matcher = new AcceptHeaderMatcher(); + $matcher = new AcceptHeaderMatcher(new ContentNegotiation()); // Remove the sample routes that would match any method. $routes = $this->fixtures->sampleRouteCollection();