Refactor REST routing to address its brittleness

Added another example, originating from #2662284-33: Return complete entity after successful PATCH.

#4: Well, it's totally valid to send data in one format and request it in a different format. And drupal will look at the Content-type header that is set for the data that comes with the request is simply untrue: it uses that format only for deserializing incoming data, not for the response. The REST module does not currently support POST/PATCH requests that have actual response bodies, and not just a status, and it's because of that that this was not noticed until #2546216: Return entity object in REST response body after successful POST and #2662284: Return complete entity after successful PATCH. So it's understandable that it's in the state it is in, but it's not correct.
EDIT: from RequestHandler::handle():

    // Deserialize incoming data if available.
    …
    if (!empty($received)) {
      $format = $request->getContentType();
      …
    }

    …

    // Invoke the operation on the resource plugin.
    // All REST routes are restricted to exactly one format, so instead of
    // parsing it out of the Accept headers again, we can simply retrieve the
    // format requirement. If there is no format associated, just pick JSON.
    $format = $route_match->getRouteObject()->getRequirement('_format') ?: 'json';
    try {
      $response = call_user_func_array(array($resource, $method), array_merge($parameters, array($unserialized, $request)));
      …

#5: I also don't know a use case for DELETE, but perhaps there is a valid use case to have a request or response body for DELETE requests? This is the one I'm least concerned about for sure.

#6: This is indeed my biggest fear, that we must continue to emulate the current bugs/quirks to not break BC.

Added more examples, originating from #2664780-54: Remove REST's resource- and verb-specific permissions for EntityResource, but provide BC and document why it's necessary for other resources.

@kriboogh: See https://www.drupal.org/node/2501221.

10. Handling of 4xx responses is done in RequestHandler::handle() instead of in a KernelEvents::EXCEPTION event subscriber. This prevents Basic Auth 401 responses from being sent, and means REST has its own system in parallel to Symfony's/Drupal core's.

This was fixed by the combination of:

1. #2739617: Make it easier to write on4xx() exception subscribers
2. #2805281: ?_format=hal_json error responses are application/json, yet should be application/hal+json
3. #2813853: RequestHandler has its own error handling rather than leaving it to KernelEvents::EXCEPTION event subscribers

One less thing to do here :)

9. RequestHandler::handle() assumes _format is always set, and always has a single value — and in case it's not set, its ultimate solution is "just pick JSON". But it's not guaranteed to have a single value… nor does it even always have a value. And in fact, PATCH/POST routes do not have _format set. So any PATCH/POST request will actually result in a JSON response, even if you explicitly specify ?_format=xml!

This was fixed by #2662284: Return complete entity after successful PATCH.

8. ResourceRoutes::alter() sets _access_rest_csrf on all routes, including GET routes. This is harmless, but it causes extra, unnecessary computations on those requests. And it makes the route more confusing to debug.

#2753681: Move CSRF header token out of REST module so that user module can use it, as well as any contrib module caused a change in this one. It's now _csrf_request_header_token. The root cause has not been addressed yet though. IS updated.

I checked it again, and points 1 through 8 all still need to be addressed.

5. ResourceBase sets _content_type_format not to the allowed formats for that particular REST resource (as per the rest.settings configuration), but to all acceptable serialization formats, and then has validation for it in \Drupal\rest\RequestHandler::handle().

This is being worked on at #2826407: PATCH + POST allowed format validation happens in RequestHandler::handle(), rather than in the routing system.

Added:

11. RequestHandler is hard to understand, and is messy. — fixed by #2853460: Simplify RequestHandler: make it no longer ContainerAware, split up ::handle()

Added:

12. RequestHandler has fairly complex logic to deal with HEAD requests. — fixed by #2775479: Try to remove the "map HEAD to GET" logic in \Drupal\rest\RequestHandler::handle()

12. RequestHandler has fairly complex logic to deal with HEAD requests. — fixed by #2775479: Try to remove the "map HEAD to GET" logic in \Drupal\rest\RequestHandler::handle()

#2775479: Try to remove the "map HEAD to GET" logic in \Drupal\rest\RequestHandler::handle() landed.

Opened #2858482: Simplify REST routing: disallow requesting POST/PATCH in any format, make consistent to fix points 1+2+3+4 and 6+7. Why fix so many points in a single issue? Because they're all very much related; they all depend on the same few lines of code in ResourceBase.

This means that only point 8 does not yet have an issue to fix it:

8. ResourceRoutes::alter() sets _csrf_request_header_token on all routes, including GET routes. This is harmless, but it causes extra, unnecessary computations on those requests. And it makes the route more confusing to debug.

I'm also removing point 13, with the ellipsis. Because once the first 12 issues are done, I think REST routing is sufficiently non-brittle.

#2858482 was RTBC, but as of #2858482-26: Simplify REST routing: disallow requesting POST/PATCH in any format, make consistent, it's now blocked on #2472337: Provide a lazy alternative to service collectors which just detects service IDs. Which means #2853460 is now blocked on two issues.

Per #25.

Note this is a blocker to #2822201: Allow contrib modules to deploy REST resources by creating routes, removing the need for config entities in that special case, which is a Contributed project soft blocker. So this is too.

#2472337: Provide a lazy alternative to service collectors which just detects service IDs landed, #2858482: Simplify REST routing: disallow requesting POST/PATCH in any format, make consistent has been rerolled and is now blocked on review. That's the next step in unblocking this!

As of #2858482-39: Simplify REST routing: disallow requesting POST/PATCH in any format, make consistent, #2858482 is blocked on another issue: #2883680: Force all route filters and route enhancers to be non-lazy. This therefore has a chain of 3 issues blocking it.

This is the only one I didn't have an issue for yet:

8. ResourceRoutes::alter() sets _csrf_request_header_token on all routes, including GET routes. This is harmless, but it causes extra, unnecessary computations on those requests. And it makes the route more confusing to debug.

But as of #2869426-28: EntityResource should add _entity_access requirement to REST routes, that now has an issue+patch, and it's actually proven to be less harmless than I thought.

#2826407: PATCH + POST allowed format validation happens in RequestHandler::handle(), rather than in the routing system landed, bringing it down to PP-2.

But as of #31, #2869426: EntityResource should add _entity_access requirement to REST routes was added, so we remain at PP-3.

Also updated #2905563: #2905563-26: REST: top priorities for Drupal 8.5.x

Forgot to strike through point 5, per #32.

#2853460: Simplify RequestHandler: make it no longer ContainerAware, split up ::handle() + #2858482: Simplify REST routing: disallow requesting POST/PATCH in any format, make consistent landed!

That only leaves #2869426: EntityResource should add _entity_access requirement to REST routes.

YAY!!!!!!! #2869426: EntityResource should add _entity_access requirement to REST routes just landed! Which means this is now done!