core/core.services.yml | 10 ++- .../Compiler/RegisterLazyRouteFilters.php | 3 +- .../Core/Routing/ContentTypeHeaderMatcher.php | 2 +- core/lib/Drupal/Core/Routing/LazyRouteEnhancer.php | 2 +- core/lib/Drupal/Core/Routing/LazyRouteFilter.php | 70 +++++++++++++++------ .../dblog/src/Tests/Rest/DbLogResourceTest.php | 6 +- core/modules/rest/src/Plugin/ResourceBase.php | 18 +++--- core/modules/rest/src/Routing/ResourceRoutes.php | 19 ++++-- core/modules/rest/src/Tests/ResourceTest.php | 3 +- ....rest-rest_post_update_resource_granularity.php | Bin 5199 -> 5233 bytes .../EntityResource/EntityResourceTestBase.php | 4 +- .../user/src/Tests/RestRegisterUserTest.php | 3 +- 12 files changed, 93 insertions(+), 47 deletions(-) diff --git a/core/core.services.yml b/core/core.services.yml index 40c6a8a..0a44bfa 100644 --- a/core/core.services.yml +++ b/core/core.services.yml @@ -936,15 +936,21 @@ services: request_format_route_filter: class: Drupal\Core\Routing\RequestFormatRouteFilter tags: + # The request format route filter must run last. - { name: route_filter } method_filter: class: Drupal\Core\Routing\MethodFilter tags: - - { name: route_filter, priority: 1 } + # The HTTP method route filter must run first: based on the request method, content type request header-based + # route filtering (content_type_header_matcher) may or may not be necessary. + - { name: route_filter, priority: 10 } content_type_header_matcher: class: Drupal\Core\Routing\ContentTypeHeaderMatcher tags: - - { name: route_filter } + # The content type request header router filter must run before the request format route filter + # (request_format_route_filter), because without a valid request body (for HTTP methods that need it, such as POST + # and PATCH), no successful response is possible. + - { name: route_filter, priority: 5 } paramconverter_manager: class: Drupal\Core\ParamConverter\ParamConverterManager tags: diff --git a/core/lib/Drupal/Core/DependencyInjection/Compiler/RegisterLazyRouteFilters.php b/core/lib/Drupal/Core/DependencyInjection/Compiler/RegisterLazyRouteFilters.php index d774779..d48f8d6 100644 --- a/core/lib/Drupal/Core/DependencyInjection/Compiler/RegisterLazyRouteFilters.php +++ b/core/lib/Drupal/Core/DependencyInjection/Compiler/RegisterLazyRouteFilters.php @@ -21,7 +21,8 @@ public function process(ContainerBuilder $container) { $service_ids = []; foreach ($container->findTaggedServiceIds('route_filter') as $id => $attributes) { - $service_ids[$id] = $id; + $priority = isset($attributes[0]['priority']) ? $attributes[0]['priority'] : 0; + $service_ids[$priority][] = $id; } $container diff --git a/core/lib/Drupal/Core/Routing/ContentTypeHeaderMatcher.php b/core/lib/Drupal/Core/Routing/ContentTypeHeaderMatcher.php index 13e18f3..4793472 100644 --- a/core/lib/Drupal/Core/Routing/ContentTypeHeaderMatcher.php +++ b/core/lib/Drupal/Core/Routing/ContentTypeHeaderMatcher.php @@ -54,7 +54,7 @@ public function filter(RouteCollection $collection, Request $request) { * {@inheritdoc} */ public function applies(Route $route) { - return TRUE; + return $route->hasRequirement('_content_type_format'); } } diff --git a/core/lib/Drupal/Core/Routing/LazyRouteEnhancer.php b/core/lib/Drupal/Core/Routing/LazyRouteEnhancer.php index 356fe3e..5299604 100644 --- a/core/lib/Drupal/Core/Routing/LazyRouteEnhancer.php +++ b/core/lib/Drupal/Core/Routing/LazyRouteEnhancer.php @@ -67,7 +67,7 @@ public function setEnhancers(RouteCollection $route_collection) { } /** - * For each route, gets a list of applicable enhancer to the route. + * Gets all lazy route enhancers. * * @return \Symfony\Cmf\Component\Routing\Enhancer\RouteEnhancerInterface[]|\Drupal\Core\Routing\Enhancer\RouteEnhancerInterface[] */ diff --git a/core/lib/Drupal/Core/Routing/LazyRouteFilter.php b/core/lib/Drupal/Core/Routing/LazyRouteFilter.php index 10f3d51..1fbc3ae 100644 --- a/core/lib/Drupal/Core/Routing/LazyRouteFilter.php +++ b/core/lib/Drupal/Core/Routing/LazyRouteFilter.php @@ -21,31 +21,35 @@ class LazyRouteFilter implements BaseRouteFilterInterface, ContainerAwareInterfa use ContainerAwareTrait; /** - * Array of route filter service IDs. + * The list of available route filters. * - * @var array + * @var \Drupal\Core\Routing\RouteFilterInterface[] + * + * @see \Drupal\Core\Routing\Router::$filters */ - protected $serviceIds = []; + protected $filters = []; /** - * The initialized route filters. + * Cached sorted list lazy route filter service IDs. * - * @var \Drupal\Core\Routing\RouteFilterInterface[] + * @var string[] + * + * @see \Drupal\Core\Routing\Router::$sortedFilters */ - protected $filters = NULL; + protected $sortedServiceIds; /** - * Constructs the LazyRouteEnhancer object. + * Constructs the LazyRouteFilter object. * * @param $service_ids - * Array of route filter service IDs. + * Array of route filter service IDs keyed by priority. */ public function __construct($service_ids) { - $this->serviceIds = $service_ids; + $this->sortedServiceIds = static::sortServices($service_ids); } /** - * For each route, filter down the route collection. + * For each route, saves a list of applicable filters to the route. * * @param \Symfony\Component\Routing\RouteCollection $route_collection * A collection of routes to apply filter checks to. @@ -66,17 +70,40 @@ public function setFilters(RouteCollection $route_collection) { } /** - * For each route, gets a list of applicable enhancers to the route. + * Gets all lazy route filters. * * @return \Symfony\Cmf\Component\Routing\Enhancer\RouteEnhancerInterface[]|\Drupal\Core\Routing\Enhancer\RouteEnhancerInterface[] */ protected function getFilters() { - if (!isset($this->filters)) { - foreach ($this->serviceIds as $service_id) { + foreach ($this->sortedServiceIds as $service_id) { + if (!isset($this->filters[$service_id])) { $this->filters[$service_id] = $this->container->get($service_id); } + yield $service_id => $this->filters[$service_id]; } - return $this->filters; + } + + /** + * Sort service IDs by priority. + * + * The highest priority number is the highest priority (reverse sorting). + * + * @param array $service_ids + * The unsorted service IDs, keyed by priority. + * @return string[] + * The sorted service IDs. + * + * @see \Drupal\Core\Routing\Router::sortFilters() + */ + protected static function sortServices(array $service_ids) { + $sorted_service_ids = []; + krsort($service_ids); + + foreach ($service_ids as $service_id) { + $sorted_service_ids = array_merge($sorted_service_ids, $service_id); + } + + return $sorted_service_ids; } /** @@ -89,12 +116,17 @@ public function filter(RouteCollection $collection, Request $request) { } $filter_ids = array_unique($filter_ids); - if (isset($filter_ids)) { - foreach ($filter_ids as $filter_id) { - if ($filter = $this->container->get($filter_id, ContainerInterface::NULL_ON_INVALID_REFERENCE)) { - $collection = $filter->filter($collection, $request); - } + if (empty($filter_ids)) { + return $collection; + } + + // Route filters are expected to throw an exception themselves if they + // end up filtering the list down to 0. + foreach ($this->getFilters() as $service_id => $filter) { + if (!in_array($service_id, $filter_ids, TRUE)) { + continue; } + $collection = $filter->filter($collection, $request); } return $collection; } diff --git a/core/modules/dblog/src/Tests/Rest/DbLogResourceTest.php b/core/modules/dblog/src/Tests/Rest/DbLogResourceTest.php index 44dbe99..abf1030 100644 --- a/core/modules/dblog/src/Tests/Rest/DbLogResourceTest.php +++ b/core/modules/dblog/src/Tests/Rest/DbLogResourceTest.php @@ -41,7 +41,7 @@ public function testWatchdog() { $account = $this->drupalCreateUser(['restful get dblog']); $this->drupalLogin($account); - $response = $this->httpRequest(Url::fromRoute('rest.dblog.GET.' . $this->defaultFormat, ['id' => $id, '_format' => $this->defaultFormat]), 'GET'); + $response = $this->httpRequest(Url::fromRoute('rest.dblog.GET', ['id' => $id, '_format' => $this->defaultFormat]), 'GET'); $this->assertResponse(200); $this->assertHeader('content-type', $this->defaultMimeType); $log = Json::decode($response); @@ -50,13 +50,13 @@ public function testWatchdog() { $this->assertEqual($log['message'], 'Test message', 'Log message text is correct.'); // Request an unknown log entry. - $response = $this->httpRequest(Url::fromRoute('rest.dblog.GET.' . $this->defaultFormat, ['id' => 9999, '_format' => $this->defaultFormat]), 'GET'); + $response = $this->httpRequest(Url::fromRoute('rest.dblog.GET', ['id' => 9999, '_format' => $this->defaultFormat]), 'GET'); $this->assertResponse(404); $decoded = Json::decode($response); $this->assertEqual($decoded['message'], 'Log entry with ID 9999 was not found', 'Response message is correct.'); // Make a bad request (a true malformed request would never be a route match). - $response = $this->httpRequest(Url::fromRoute('rest.dblog.GET.' . $this->defaultFormat, ['id' => 0, '_format' => $this->defaultFormat]), 'GET'); + $response = $this->httpRequest(Url::fromRoute('rest.dblog.GET', ['id' => 0, '_format' => $this->defaultFormat]), 'GET'); $this->assertResponse(400); $decoded = Json::decode($response); $this->assertEqual($decoded['message'], 'No log entry ID was provided', 'Response message is correct.'); diff --git a/core/modules/rest/src/Plugin/ResourceBase.php b/core/modules/rest/src/Plugin/ResourceBase.php index 3062aa2..459a290 100644 --- a/core/modules/rest/src/Plugin/ResourceBase.php +++ b/core/modules/rest/src/Plugin/ResourceBase.php @@ -111,35 +111,31 @@ public function routes() { switch ($method) { case 'POST': $route->setPath($create_path); + $route->addRequirements(['_format' => implode('|', $this->serializerFormats)]); // Restrict the incoming HTTP Content-type header to the known // serialization formats. $route->addRequirements(['_content_type_format' => implode('|', $this->serializerFormats)]); - $collection->add("$route_name.$method", $route); break; case 'PATCH': + $route->addRequirements(['_format' => implode('|', $this->serializerFormats)]); // Restrict the incoming HTTP Content-type header to the known // serialization formats. $route->addRequirements(['_content_type_format' => implode('|', $this->serializerFormats)]); - $collection->add("$route_name.$method", $route); break; case 'GET': case 'HEAD': - // Restrict GET and HEAD requests to the media type specified in the - // HTTP Accept headers. - foreach ($this->serializerFormats as $format_name) { - // Expose one route per available format. - $format_route = clone $route; - $format_route->addRequirements(['_format' => $format_name]); - $collection->add("$route_name.$method.$format_name", $format_route); - } + $route->addRequirements(['_format' => implode('|', $this->serializerFormats)]); break; + case 'DELETE': + // No request body nor response body for DELETE requests, so also no + // need for a '_format' or '_content_type_format' requirement. default: - $collection->add("$route_name.$method", $route); break; } + $collection->add("$route_name.$method", $route); } return $collection; diff --git a/core/modules/rest/src/Routing/ResourceRoutes.php b/core/modules/rest/src/Routing/ResourceRoutes.php index 6aec267..4527d29 100644 --- a/core/modules/rest/src/Routing/ResourceRoutes.php +++ b/core/modules/rest/src/Routing/ResourceRoutes.php @@ -106,11 +106,20 @@ protected function getRoutesForResourceConfig(RestResourceConfigInterface $rest_ continue; } - // If the route has a format requirement, then verify that the - // resource has it. - $format_requirement = $route->getRequirement('_format'); - if ($format_requirement && !in_array($format_requirement, $rest_resource_config->getFormats($method))) { - continue; + // If the route has a format requirement, then verify that it matches + // the REST resource config. Remove formats that are not listed in the + // formats for this method in the REST resource config. + // @todo Remove this in Drupal 9. ResourceInterface::routes() should be + // given the REST resource config entity, and should generate the + // appropriate routes based on that. But doing so would be a BC + // break. This is the next best thing we can do. + if ($route->hasRequirement('_format')) { + $format_requirements = explode('|', $route->getRequirement('_format')); + $allowed_formats = array_intersect($format_requirements, $rest_resource_config->getFormats($method)); + if (empty($allowed_formats)) { + continue; + } + $route->setRequirement('_format', implode('|', $allowed_formats)); } // The configuration seems legit at this point, so we set the diff --git a/core/modules/rest/src/Tests/ResourceTest.php b/core/modules/rest/src/Tests/ResourceTest.php index baf91e9..38e6330 100644 --- a/core/modules/rest/src/Tests/ResourceTest.php +++ b/core/modules/rest/src/Tests/ResourceTest.php @@ -3,6 +3,7 @@ namespace Drupal\rest\Tests; use Drupal\Core\Session\AccountInterface; +use Drupal\Core\Url; use Drupal\rest\RestResourceConfigInterface; use Drupal\user\Entity\Role; use Drupal\user\RoleInterface; @@ -107,7 +108,7 @@ public function testSerializationClassIsOptional() { ->save(); $serialized = $this->container->get('serializer')->serialize(['foo', 'bar'], 'json'); - $this->httpRequest('serialization_test', 'POST', $serialized, 'application/json'); + $this->httpRequest(Url::fromRoute('rest.serialization_test.POST', ['_format' => 'json']), 'POST', $serialized, 'application/json'); $this->assertResponse(200); $this->assertResponseBody('["foo","bar"]'); } diff --git a/core/modules/rest/tests/fixtures/update/drupal-8.rest-rest_post_update_resource_granularity.php b/core/modules/rest/tests/fixtures/update/drupal-8.rest-rest_post_update_resource_granularity.php index aa94c63e555977ccc5ada2d04fd8632562e09a14..989f57927209b7c3bdc84818df5ee0f8ab177aef 100644 GIT binary patch delta 20 ccmX@F@lj*LH74ea#GJ{unB+HKW2zJc0APg(bN~PV delta 12 TcmeyUab9D?HKxr!m?{MUDqIE7 diff --git a/core/modules/rest/tests/src/Functional/EntityResource/EntityResourceTestBase.php b/core/modules/rest/tests/src/Functional/EntityResource/EntityResourceTestBase.php index 53796c8..500cfff 100644 --- a/core/modules/rest/tests/src/Functional/EntityResource/EntityResourceTestBase.php +++ b/core/modules/rest/tests/src/Functional/EntityResource/EntityResourceTestBase.php @@ -544,7 +544,7 @@ public function testGet() { $this->assertSame(['text/plain; charset=UTF-8'], $response->getHeader('Content-Type')); - $url = Url::fromRoute('rest.entity.' . static::$entityTypeId . '.GET.' . static::$format); + $url = Url::fromRoute('rest.entity.' . static::$entityTypeId . '.GET'); $url->setRouteParameter(static::$entityTypeId, 987654321); $url->setOption('query', ['_format' => static::$format]); @@ -552,7 +552,7 @@ public function testGet() { // DX: 404 when GETting non-existing entity. $response = $this->request('GET', $url, $request_options); $path = str_replace('987654321', '{' . static::$entityTypeId . '}', $url->setAbsolute()->setOptions(['base_url' => '', 'query' => []])->toString()); - $message = 'The "' . static::$entityTypeId . '" parameter was not converted for the path "' . $path . '" (route name: "rest.entity.' . static::$entityTypeId . '.GET.' . static::$format . '")'; + $message = 'The "' . static::$entityTypeId . '" parameter was not converted for the path "' . $path . '" (route name: "rest.entity.' . static::$entityTypeId . '.GET")'; $this->assertResourceErrorResponse(404, $message, $response); } diff --git a/core/modules/user/src/Tests/RestRegisterUserTest.php b/core/modules/user/src/Tests/RestRegisterUserTest.php index 62e6bbc..3461a82 100644 --- a/core/modules/user/src/Tests/RestRegisterUserTest.php +++ b/core/modules/user/src/Tests/RestRegisterUserTest.php @@ -2,6 +2,7 @@ namespace Drupal\user\Tests; +use Drupal\Core\Url; use Drupal\rest\Tests\RESTTestBase; use Drupal\user\Entity\Role; use Drupal\user\RoleInterface; @@ -166,7 +167,7 @@ protected function registerUser($name, $include_password = TRUE) { */ protected function registerRequest($name, $include_password = TRUE) { $serialized = $this->createSerializedUser($name, $include_password); - $this->httpRequest('/user/register', 'POST', $serialized, 'application/hal+json'); + $this->httpRequest(Url::fromRoute('rest.user_registration.POST', ['_format' => 'hal_json']), 'POST', $serialized, 'application/hal+json'); } }