Requests are pushed onto the request stack twice, popped once

few TODO added in 📌 Add the session to the request in KernelTestBase, BrowserTestBase, and drush Fixed

Rerolled.
Changes affected by CR Drupal\Core\Http\RequestStack is deprecated →
Created MR

Thank you, some tests failed and it needs CR

As there's many+1 in related issue it could be follow-up to prevent popping of ech request twice

Fixed tests after https://www.drupal.org/node/3357408 →

2 tests still fail

I've looked at the MR and the current solution is quite confusing for me. I wonder if instead of having a wrapped request stack we can pass in a NULL for the request stack and allow the Symfony HttpKernel to use it's own request stack - separate from Drupals. This would decouple the request stacks which I think might make this make more sense than wrapping.

Alternatively we could admit the \Symfony\Component\HttpKernel\HttpKernel doesn't work for us and we could copy and maintain the code in a way that does.

So the idea of just allowing the Symfony code to instantiate its own request stack did not work - see https://git.drupalcode.org/project/drupal/-/merge_requests/9861 - interesting fails.

I've updated the old MR to pass PHPStan and coding standards on 11.x plus I've made the service it adds private and less useful looking. I'm still not convinced that adding a new service is better than reworking Symfony\Component\HttpKernel\HttpKernel to work the way we want it to.

> Alternatively we could admit the \Symfony\Component\HttpKernel\HttpKernel doesn't work for us and we could copy and maintain the code in a way that does.

The fact that we use the Symfony HttpKernel is pretty useful over at ✨ Add drupalGet() to KernelTestBase Active .

Hit a pretty gnarly bug from this yesterday on a client project that was making use of sub-requests and cacheable json responses.

The net result was cache poisoning via the route cache context.

With a sub-request we push each request to the stack twice but only pop it once, the current route match can end up still pointing at the sub-request rather than the parent request and as a result any cacheable responses that use the route cache context end up with the wrong route name and parameters.

This lead to a scenario where an anonymous user could request a JSON page that made a subrequest to a regular node page but the JSON page ended up being cached with the node page's route params. After that any request to the node page was getting the cached JSON response.

What's left here?

What's wrong with the original MR that tries to pop off the extra request?