Prevent caching of tracker in proxies (#12730)

* Prevent caching of tracker in proxies

* Also send Cache-Control with 204 responses.

Varnish default config violates the HTTP spec in this respect.

* Test Cache-Control header

* Update changelog

* Use no-store instead of no-cache

4 files changed, 49 insertions, 15 deletions

diff --git a/tests/PHPUnit/Framework/Constraint/HttpResponseText.php b/tests/PHPUnit/Framework/Constraint/HttpResponseText.php
index 223d067a3a..2225d9daf3 100644
--- a/tests/PHPUnit/Framework/Constraint/HttpResponseText.php
+++ b/tests/PHPUnit/Framework/Constraint/HttpResponseText.php
@@ -7,6 +7,9 @@
  */
 namespace Piwik\Tests\Framework\Constraint;
 
+/**
+ * @deprecated
+ */
 class HttpResponseText extends \PHPUnit_Framework_Constraint
 {
     private $actualCode;
@@ -60,4 +63,4 @@ class HttpResponseText extends \PHPUnit_Framework_Constraint
     {
         return 'does not return response text ' . $this->exporter->export($this->value) . ' it is ' . $this->actualCode;
     }
-}?>
\ No newline at end of file
+}?>
diff --git a/tests/PHPUnit/Framework/Constraint/ResponseCode.php b/tests/PHPUnit/Framework/Constraint/ResponseCode.php
index c23b136a82..fc0cae2f97 100644
--- a/tests/PHPUnit/Framework/Constraint/ResponseCode.php
+++ b/tests/PHPUnit/Framework/Constraint/ResponseCode.php
@@ -7,6 +7,9 @@
  */
 namespace Piwik\Tests\Framework\Constraint;
 
+/**
+ * @deprecated
+ */
 class ResponseCode extends \PHPUnit_Framework_Constraint
 {
     private $actualCode;
@@ -56,4 +59,4 @@ class ResponseCode extends \PHPUnit_Framework_Constraint
     {
         return 'does not return response code ' . $this->exporter->export($this->value) . ' it is ' . $this->actualCode;
     }
-}?>
\ No newline at end of file
+}?>
diff --git a/tests/PHPUnit/Framework/TestCase/SystemTestCase.php b/tests/PHPUnit/Framework/TestCase/SystemTestCase.php
index eb051f04fa..fef22f1d93 100755
--- a/tests/PHPUnit/Framework/TestCase/SystemTestCase.php
+++ b/tests/PHPUnit/Framework/TestCase/SystemTestCase.php
@@ -703,11 +703,17 @@ abstract class SystemTestCase extends PHPUnit_Framework_TestCase
         DbHelper::deleteArchiveTables();
     }
 
+    /**
+     * @deprecated
+     */
     public function assertHttpResponseText($expectedResponseText, $url, $message = '')
     {
         self::assertThat($url, new HttpResponseText($expectedResponseText), $message);
     }
 
+    /**
+     * @deprecated
+     */
     public function assertResponseCode($expectedResponseCode, $url, $message = '')
     {
         self::assertThat($url, new ResponseCode($expectedResponseCode), $message);
diff --git a/tests/PHPUnit/System/TrackerResponseTest.php b/tests/PHPUnit/System/TrackerResponseTest.php
index 1c24790b29..35d9a548ce 100755
--- a/tests/PHPUnit/System/TrackerResponseTest.php
+++ b/tests/PHPUnit/System/TrackerResponseTest.php
@@ -7,6 +7,7 @@
  */
 namespace Piwik\Tests\System;
 
+use Piwik\Http;
 use Piwik\Tests\Framework\Fixture;
 use Piwik\Tests\Framework\TestCase\SystemTestCase;
 
@@ -58,7 +59,10 @@ class TrackerResponseTest extends SystemTestCase
     {
         $url = $this->tracker->getUrlTrackPageView('Test');
 
-        $this->assertResponseCode(200, $url);
+        $response = $this->sendHttpRequest($url);
+        $this->assertEquals(200, $response['status']);
+        $this->assertArrayHasKey('Cache-Control', $response['headers']);
+        $this->assertEquals('no-store', $response['headers']['Cache-Control']);
     }
 
     public function test_response_ShouldSend204ResponseCode_IfImageIsDisabled()
@@ -66,7 +70,10 @@ class TrackerResponseTest extends SystemTestCase
         $url = $this->tracker->getUrlTrackPageView('Test');
         $url .= '&send_image=0';
 
-        $this->assertResponseCode(204, $url);
+        $response = $this->sendHttpRequest($url);
+        $this->assertEquals(204, $response['status']);
+        $this->assertArrayHasKey('Cache-Control', $response['headers']);
+        $this->assertEquals('no-store', $response['headers']['Cache-Control']);
     }
 
     public function test_response_ShouldSend400ResponseCode_IfSiteIdIsInvalid()
@@ -74,7 +81,8 @@ class TrackerResponseTest extends SystemTestCase
         $url = $this->tracker->getUrlTrackPageView('Test');
         $url .= '&idsite=100';
 
-        $this->assertResponseCode(400, $url);
+        $response = $this->sendHttpRequest($url);
+        $this->assertEquals(400, $response['status']);
     }
 
     public function test_response_ShouldSend400ResponseCode_IfSiteIdIsZero()
@@ -82,7 +90,8 @@ class TrackerResponseTest extends SystemTestCase
         $url = $this->tracker->getUrlTrackPageView('Test');
         $url .= '&idsite=0';
 
-        $this->assertResponseCode(400, $url);
+        $response = $this->sendHttpRequest($url);
+        $this->assertEquals(400, $response['status']);
     }
 
     public function test_response_ShouldSend400ResponseCode_IfInvalidRequestParameterIsGiven()
@@ -90,8 +99,13 @@ class TrackerResponseTest extends SystemTestCase
         $url = $this->tracker->getUrlTrackPageView('Test');
         $url .= '&cid=' . str_pad('1', 16, '1');
 
-        $this->assertResponseCode(200, $url);
-        $this->assertResponseCode(400, $url . '1'); // has to be 16 char, but is 17 now
+        $response = $this->sendHttpRequest($url);
+        $this->assertArrayHasKey('Cache-Control', $response['headers']);
+        $this->assertEquals('no-store', $response['headers']['Cache-Control']);
+        $this->assertEquals(200, $response['status']);
+
+        $response = $this->sendHttpRequest($url . '1'); // has to be 16 char, but is 17 now
+        $this->assertEquals(400, $response['status']);
     }
 
     // See https://github.com/piwik/piwik/issues/7850 piwik.php is used by plugins and monitoring systems to test for Piwik installation.
@@ -99,29 +113,37 @@ class TrackerResponseTest extends SystemTestCase
     public function test_response_ShouldReturnPiwikMessageWithHttp200_InCaseOfEmptyGETRequest()
     {
         $url = Fixture::getTrackerUrl();
-        $this->assertResponseCode(200, $url);
+        $response = Http::sendHttpRequest($url, 10, null, null, 0, false, false, true);
+        $this->assertEquals(200, $response['status']);
 
         $expected = "This resource is part of Matomo. Keep full control of your data with the leading free and open source <a href='https://matomo.org' target='_blank' rel='noopener noreferrer'>digital analytics platform</a> for web and mobile.";
-        $this->assertHttpResponseText($expected, $url);
+        $this->assertEquals($expected, $response['data']);
     }
 
     public function test_response_ShouldReturnPiwikMessageWithHttp400_InCaseOfInvalidRequestOrIfNothingIsTracked()
     {
         $url = Fixture::getTrackerUrl();
-        $this->assertResponseCode(400, $url . '?rec=1');
+        $response = $this->sendHttpRequest($url . '?rec=1');
+        $this->assertEquals(400, $response['status']);
 
+        $response = $this->sendHttpRequest($url);
         $expected = "This resource is part of Matomo. Keep full control of your data with the leading free and open source <a href='https://matomo.org' target='_blank' rel='noopener noreferrer'>digital analytics platform</a> for web and mobile.";
-        $this->assertHttpResponseText($expected, $url);
+        $this->assertEquals($expected, $response['data']);
     }
 
-
     public function test_response_ShouldReturnPiwikMessageWithHttp503_InCaseOfMaintenanceMode()
     {
         $url = $this->tracker->getUrlTrackPageView('Test');
-        $this->assertResponseCode(200, $url);
+        $response = $this->sendHttpRequest($url);
+        $this->assertEquals(200, $response['status']);
 
         $url = $url . "&forceEnableTrackerMaintenanceMode=1";
-        $this->assertResponseCode(503, $url);
+        $response = $this->sendHttpRequest($url);
+        $this->assertEquals(503, $response['status']);
     }
 
+    protected function sendHttpRequest($url)
+    {
+        return Http::sendHttpRequest($url, 10, null, null, 0, false, false, true);
+    }
 }