Cookies get lost during RedirectResponse replacement

Could use an automated test to ensure we don't run into this bug in the future.

Wow, is that something we could fix upstream?

Thank you for adding tests! I agree, PIFR your reign is over.

Fixable on commit:

1. +++ b/core/tests/Drupal/Tests/Component/HttpFoundation/SecuredRedirectResponseTest.php
   @@ -0,0 +1,62 @@
   +   *  {@inheritdoc}
   

   Extra space between * and {

2. +++ b/core/tests/Drupal/Tests/Component/HttpFoundation/SecuredRedirectResponseTest.php
   @@ -0,0 +1,62 @@
   +    return true;
   

   return TRUE;

+++ b/core/lib/Drupal/Component/HttpFoundation/SecuredRedirectResponse.php
@@ -35,6 +35,9 @@ public static function createFromRedirectResponse(RedirectResponse $response) {
     $safe_response->setProtocolVersion($response->getProtocolVersion());
     $safe_response->setCharset($response->getCharset());
+    foreach ($response->headers->getCookies() as $cookie) {
+      $safe_response->headers->setCookie($cookie);
+    }
     return $safe_response;

Can we have a comment why we are doing this here?

Thank you re-RTBC. Comments and docs addressed.

@joelpittet
https://www.drupal.org/core/d8-allowed-changes
says

Non-criticals under consideration have the rc target triage tag. Issues tagged with rc target triage should have an issue summary that explains why it is important for the issue to land during RC and why it can not wait until later.

I think we need a "beta evaluation" like thing in the issue summary which goes through the criteria in https://www.drupal.org/core/d8-allowed-changes

Not sure how this will play out, I'm sure if it doesn't get in for RC it can likely go into 8.0.1 or something, but there is a fancy RC evaluation in the IS now.

+++ b/core/lib/Drupal/Component/HttpFoundation/SecuredRedirectResponse.php
@@ -46,6 +46,11 @@ public static function createFromRedirectResponse(RedirectResponse $response) {
+    // Cookies aren't are separate from other headers and have to be copied over
+    // directly.

nit: remove the word 'arn't'

Would you mind posting a tests only patch with the next patch?

Thank you @neclimdul. I think this has all the right stuff now.

Discussed with the D8 committers and we agreed with making this an rc target.

Committed and pushed to 8.0.x. Thanks!