Merge pull request #4325 from dhensby/pulls/http-fix

Fixing issues with HTTP cache control
silverstripe · Jul 23, 2015 · 7781619 · 7781619
2 parents 639ebef + 33d93c2
commit 7781619
Show file tree

Hide file tree

Showing 2 changed files with 28 additions and 9 deletions.
diff --git a/control/HTTP.php b/control/HTTP.php
@@ -308,7 +308,7 @@ public static function register_etag($etag) {
 	/**
 	 * Add the appropriate caching headers to the response, including If-Modified-Since / 304 handling.
 	 *
-	 * @param SS_HTTPResponse The SS_HTTPResponse object to augment.  Omitted the argument or passing a string is
+	 * @param SS_HTTPResponse $body The SS_HTTPResponse object to augment.  Omitted the argument or passing a string is
 	 *                            deprecated; in these cases, the headers are output directly.
 	 */
 	public static function add_cache_headers($body = null) {
@@ -328,21 +328,17 @@ public static function add_cache_headers($body = null) {
 		// us trying.
 		if(headers_sent() && !$body) return;
 
-		// Popuplate $responseHeaders with all the headers that we want to build
+		// Populate $responseHeaders with all the headers that we want to build
 		$responseHeaders = array();
 
 		$config = Config::inst();
 		$cacheControlHeaders = Config::inst()->get('HTTP', 'cache_control');
 
 
-		// currently using a config setting to cancel this, seems to be so taht the CMS caches ajax requests
+		// currently using a config setting to cancel this, seems to be so that the CMS caches ajax requests
 		if(function_exists('apache_request_headers') && $config->get(get_called_class(), 'cache_ajax_requests')) {
-			$requestHeaders = apache_request_headers();
+			$requestHeaders = array_change_key_case(apache_request_headers(), CASE_LOWER);
 
-			if(isset($requestHeaders['X-Requested-With']) && $requestHeaders['X-Requested-With']=='XMLHttpRequest') {
-				$cacheAge = 0;
-			}
-			// bdc: now we must check for DUMB IE6:
 			if(isset($requestHeaders['x-requested-with']) && $requestHeaders['x-requested-with']=='XMLHttpRequest') {
 				$cacheAge = 0;
 			}
@@ -383,13 +379,16 @@ public static function add_cache_headers($body = null) {
 		foreach($cacheControlHeaders as $header => $value) {
 			if(is_null($value)) {
 				unset($cacheControlHeaders[$header]);
-			} elseif(is_bool($value) || $value === "true") {
+			} elseif((is_bool($value) && $value) || $value === "true") {
 				$cacheControlHeaders[$header] = $header;
 			} else {
 				$cacheControlHeaders[$header] = $header."=".$value;
 			}
 		}
 
+		$responseHeaders['Cache-Control'] = implode(', ', $cacheControlHeaders);
+		unset($cacheControlHeaders, $header, $value);
+
 		if(self::$modification_date && $cacheAge > 0) {
 			$responseHeaders["Last-Modified"] = self::gmt_date(self::$modification_date);
 

diff --git a/tests/control/HTTPTest.php b/tests/control/HTTPTest.php
@@ -7,6 +7,26 @@
  */
 class HTTPTest extends FunctionalTest {
 
+	public function testAddCacheHeaders() {
+		$body = "<html><head></head><body><h1>Mysite</h1></body></html>";
+		$response = new SS_HTTPResponse($body, 200);
+		$this->assertEmpty($response->getHeader('Cache-Control'));
+
+		HTTP::set_cache_age(30);
+		HTTP::add_cache_headers($response);
+
+		$this->assertNotEmpty($response->getHeader('Cache-Control'));
+
+		Config::inst()->update('Director', 'environment_type', 'dev');
+		HTTP::add_cache_headers($response);
+		$this->assertContains('max-age=0', $response->getHeader('Cache-Control'));
+
+		Config::inst()->update('Director', 'environment_type', 'live');
+		HTTP::add_cache_headers($response);
+		$this->assertContains('max-age=30', explode(', ', $response->getHeader('Cache-Control')));
+		$this->assertNotContains('max-age=0', $response->getHeader('Cache-Control'));
+	}
+
 	/**
 	 * Tests {@link HTTP::getLinksIn()}
 	 */