From b5bae137bd341eeda3f4886f45fc8f8d657a9c4c Mon Sep 17 00:00:00 2001
From: Loz Calver <lozcalver@bigfork.co.uk>
Date: Thu, 15 Nov 2018 10:18:54 +0000
Subject: [PATCH] FIX: Redirect loop with multiple confirmation tokens present
 (fixes #8607)

---
 src/Core/Startup/ConfirmationTokenChain.php          |  5 +++--
 .../php/Core/Startup/ConfirmationTokenChainTest.php  | 12 +++++++-----
 .../Core/Startup/ErrorControlChainMiddlewareTest.php |  4 ++--
 3 files changed, 12 insertions(+), 9 deletions(-)

diff --git a/src/Core/Startup/ConfirmationTokenChain.php b/src/Core/Startup/ConfirmationTokenChain.php
index a47f2c4c0..e042bb6cb 100644
--- a/src/Core/Startup/ConfirmationTokenChain.php
+++ b/src/Core/Startup/ConfirmationTokenChain.php
@@ -139,9 +139,10 @@ class ConfirmationTokenChain
      */
     public function getRedirectUrlParams()
     {
-        $params = [];
+        $params = $_GET;
+        unset($params['url']); // CLIRequestBuilder may add this
         foreach ($this->filteredTokens() as $token) {
-            $params = array_merge($params, $token->getRedirectUrlParams());
+            $params = array_merge($params, $token->params());
         }
 
         return $params;
diff --git a/tests/php/Core/Startup/ConfirmationTokenChainTest.php b/tests/php/Core/Startup/ConfirmationTokenChainTest.php
index adb8fba36..33c843279 100644
--- a/tests/php/Core/Startup/ConfirmationTokenChainTest.php
+++ b/tests/php/Core/Startup/ConfirmationTokenChainTest.php
@@ -167,19 +167,21 @@ class ConfirmationTokenChainTest extends SapphireTest
 
     public function testGetRedirectUrlParams()
     {
-        $mockToken = $this->getTokenRequiringReload(true, ['getRedirectUrlParams']);
+        $mockToken = $this->getTokenRequiringReload(true, ['params']);
         $mockToken->expects($this->once())
-            ->method('getRedirectUrlParams')
+            ->method('params')
             ->will($this->returnValue(['mockTokenParam' => '1']));
 
-        $secondMockToken = $this->getTokenRequiringReload(true, ['getRedirectUrlParams']);
+        $secondMockToken = $this->getTokenRequiringReload(true, ['params']);
         $secondMockToken->expects($this->once())
-            ->method('getRedirectUrlParams')
+            ->method('params')
             ->will($this->returnValue(['secondMockTokenParam' => '2']));
 
         $chain = new ConfirmationTokenChain();
         $chain->pushToken($mockToken);
         $chain->pushToken($secondMockToken);
-        $this->assertEquals(['mockTokenParam' => '1', 'secondMockTokenParam' => '2'], $chain->getRedirectUrlParams());
+        $params = $chain->getRedirectUrlParams();
+        $this->assertEquals('1', $params['mockTokenParam']);
+        $this->assertEquals('2', $params['secondMockTokenParam']);
     }
 }
diff --git a/tests/php/Core/Startup/ErrorControlChainMiddlewareTest.php b/tests/php/Core/Startup/ErrorControlChainMiddlewareTest.php
index 7cf793c2f..63b47b8d7 100644
--- a/tests/php/Core/Startup/ErrorControlChainMiddlewareTest.php
+++ b/tests/php/Core/Startup/ErrorControlChainMiddlewareTest.php
@@ -49,7 +49,7 @@ class ErrorControlChainMiddlewareTest extends SapphireTest
 
         $this->assertInstanceOf(HTTPResponse::class, $result);
         $location = $result->getHeader('Location');
-        $this->assertContains('?flush=1&flushtoken=', $location);
+        $this->assertContains('flush=1&flushtoken=', $location);
         $this->assertNotContains('Security/login', $location);
     }
 
@@ -96,7 +96,7 @@ class ErrorControlChainMiddlewareTest extends SapphireTest
         $this->assertInstanceOf(HTTPResponse::class, $result);
         $location = $result->getHeader('Location');
         $this->assertContains('/dev/build', $location);
-        $this->assertContains('?devbuildtoken=', $location);
+        $this->assertContains('devbuildtoken=', $location);
         $this->assertNotContains('Security/login', $location);
     }