Ensure that duplicate cache-control header is a warning

silverstripe · Jun 7, 2018 · 1cb8a39 · 1cb8a39
1 parent 14ba6ed
commit 1cb8a39
Show file tree

Hide file tree

Showing 4 changed files with 7 additions and 11 deletions.
diff --git a/api/RSSFeed.php b/api/RSSFeed.php
@@ -204,10 +204,7 @@ public function outputToBrowser() {
 			HTTP::register_etag($this->etag);
 		}
 
-		if(!headers_sent()) {
-			HTTP::add_cache_headers();
-			$response->addHeader("Content-Type", "application/rss+xml; charset=utf-8");
-		}
+		$response->addHeader("Content-Type", "application/rss+xml; charset=utf-8");
 
 		Config::inst()->update('SSViewer', 'source_file_comments', $prevState);
 

diff --git a/control/Controller.php b/control/Controller.php
@@ -169,10 +169,7 @@ public function handleRequest(SS_HTTPRequest $request, DataModel $model) {
 			$response->setBody($body);
 		}
 
-
 		ContentNegotiator::process($response);
-		HTTP::add_cache_headers($response);
-
 		$this->popCurrent();
 		return $response;
 	}

diff --git a/control/HTTP.php b/control/HTTP.php
@@ -371,8 +371,10 @@ public static function add_cache_headers($body = null) {
 		if(headers_sent() && !$body) {
 			return;
 		}
-		// Skip already assigned cache-control headers
+
+		// Warn if already assigned cache-control headers
 		if ($body && $body->getHeader('Cache-Control')) {
+			trigger_error("Cache-Control header has already been set", E_USER_WARNING);
 			return;
 		}
 

diff --git a/tests/control/HTTPTest.php b/tests/control/HTTPTest.php
@@ -57,10 +57,10 @@ public function testAddCacheHeaders() {
 		foreach($headers as $name => $value) {
 			$response->addHeader($name, $value);
 		}
+
+		// Expect a warning if the header is already set
+		$this->setExpectedException('PHPUnit_Framework_Error_Warning', 'Cache-Control header has already been set');
 		HTTP::add_cache_headers($response);
-		foreach($headers as $name => $value) {
-			$this->assertEquals($value, $response->getHeader($name));
-		}
 	}
 
     public function testConfigVary() {