From f952ef747b2e6d86717939edb4008283d71e8a74 Mon Sep 17 00:00:00 2001
From: Colin Tucker <colin@praxis.net.au>
Date: Tue, 31 Oct 2017 20:17:55 +1100
Subject: [PATCH] Fixed array/object mismatch bug in PaginatedList

---
 src/Control/HTTP.php           |  2 +-
 src/ORM/PaginatedList.php      | 27 ++++++++++++++++-----------
 tests/php/Control/HTTPTest.php |  6 +++---
 3 files changed, 20 insertions(+), 15 deletions(-)

diff --git a/src/Control/HTTP.php b/src/Control/HTTP.php
index 1cff05eb2..cd407bdd7 100644
--- a/src/Control/HTTP.php
+++ b/src/Control/HTTP.php
@@ -167,7 +167,7 @@ class HTTP
      * @param string $separator Separator for http_build_query().
      * @return string
      */
-    public static function setGetVar($varname, $varvalue, $currentURL = null, $separator = '&amp;')
+    public static function setGetVar($varname, $varvalue, $currentURL = null, $separator = '&')
     {
         if (!isset($currentURL)) {
             $request = Controller::curr()->getRequest();
diff --git a/src/ORM/PaginatedList.php b/src/ORM/PaginatedList.php
index 646075c94..7df431e5a 100644
--- a/src/ORM/PaginatedList.php
+++ b/src/ORM/PaginatedList.php
@@ -3,6 +3,7 @@
 namespace SilverStripe\ORM;
 
 use SilverStripe\Control\HTTP;
+use SilverStripe\Control\HTTPRequest;
 use SilverStripe\ORM\Queries\SQLSelect;
 use SilverStripe\View\ArrayData;
 use ArrayAccess;
@@ -259,7 +260,11 @@ class PaginatedList extends ListDecorator
         for ($i = $start; $i < $end; $i++) {
             $result->push(new ArrayData(array(
                 'PageNum' => $i + 1,
-                'Link' => HTTP::setGetVar($this->getPaginationGetVar(), $i * $this->getPageLength()),
+                'Link' => HTTP::setGetVar(
+                    $this->getPaginationGetVar(),
+                    $i * $this->getPageLength(),
+                    ($this->request instanceof HTTPRequest) ? $this->request->getURL(true) : null
+                ),
                 'CurrentBool' => $this->CurrentPage() == ($i + 1)
             )));
         }
@@ -330,7 +335,11 @@ class PaginatedList extends ListDecorator
         }
 
         for ($i = 0; $i < $total; $i++) {
-            $link = HTTP::setGetVar($this->getPaginationGetVar(), $i * $this->getPageLength());
+            $link = HTTP::setGetVar(
+                $this->getPaginationGetVar(),
+                $i * $this->getPageLength(),
+                ($this->request instanceof HTTPRequest) ? $this->request->getURL(true) : null
+            );
             $num = $i + 1;
 
             $emptyRange = $num != 1 && $num != $total && (
@@ -439,8 +448,7 @@ class PaginatedList extends ListDecorator
         return HTTP::setGetVar(
             $this->getPaginationGetVar(),
             0,
-            $this->request ? $this->request->getURL(true) : null,
-            '&'
+            ($this->request instanceof HTTPRequest) ? $this->request->getURL(true) : null
         );
     }
 
@@ -454,8 +462,7 @@ class PaginatedList extends ListDecorator
         return HTTP::setGetVar(
             $this->getPaginationGetVar(),
             ($this->TotalPages() - 1) * $this->getPageLength(),
-            $this->request ? $this->request->getURL(true) : null,
-            '&'
+            ($this->request instanceof HTTPRequest) ? $this->request->getURL(true) : null
         );
     }
 
@@ -471,8 +478,7 @@ class PaginatedList extends ListDecorator
             return HTTP::setGetVar(
                 $this->getPaginationGetVar(),
                 $this->getPageStart() + $this->getPageLength(),
-                $this->request ? $this->request->getURL(true) : null,
-                '&'
+                ($this->request instanceof HTTPRequest) ? $this->request->getURL(true) : null
             );
         }
     }
@@ -489,8 +495,7 @@ class PaginatedList extends ListDecorator
             return HTTP::setGetVar(
                 $this->getPaginationGetVar(),
                 $this->getPageStart() - $this->getPageLength(),
-                $this->request ? $this->request->getURL(true) : null,
-                '&'
+                ($this->request instanceof HTTPRequest) ? $this->request->getURL(true) : null
             );
         }
     }
@@ -506,7 +511,7 @@ class PaginatedList extends ListDecorator
     /**
      * Set the request object for this list
      *
-     * @param HTTPRequest
+     * @param HTTPRequest|ArrayAccess
      */
     public function setRequest($request)
     {
diff --git a/tests/php/Control/HTTPTest.php b/tests/php/Control/HTTPTest.php
index 20cf0eb75..fdc3cec4b 100644
--- a/tests/php/Control/HTTPTest.php
+++ b/tests/php/Control/HTTPTest.php
@@ -154,13 +154,13 @@ class HTTPTest extends FunctionalTest
         );
 
         $this->assertEquals(
-            'relative/url?baz=buz&amp;foo=bar',
+            'relative/url?baz=buz&foo=bar',
             HTTP::setGetVar('foo', 'bar', '/relative/url?baz=buz'),
             'Relative URL with existing query params, and new added key'
         );
 
         $this->assertEquals(
-            'http://test.com/?foo=new&amp;buz=baz',
+            'http://test.com/?foo=new&buz=baz',
             HTTP::setGetVar('foo', 'new', 'http://test.com/?foo=old&buz=baz'),
             'Absolute URL without path and multipe existing query params, overwriting an existing parameter'
         );
@@ -172,7 +172,7 @@ class HTTPTest extends FunctionalTest
         );
         // http_build_query() escapes angular brackets, they should be correctly urldecoded by the browser client
         $this->assertEquals(
-            'http://test.com/?foo%5Btest%5D=one&amp;foo%5Btest%5D=two',
+            'http://test.com/?foo%5Btest%5D=one&foo%5Btest%5D=two',
             HTTP::setGetVar('foo[test]', 'two', 'http://test.com/?foo[test]=one'),
             'Absolute URL and PHP array query string notation'
         );