From f192a6ecaf70446ec60f6c7ef2a555395f83ea16 Mon Sep 17 00:00:00 2001
From: Patrick Nelson <patnelson@ebay.com>
Date: Sun, 12 Jul 2015 04:36:39 -0400
Subject: [PATCH] FIX #4392: Ensure headers are checked first before being
 clobbered by globally maintained state. Also ensuring tests utilize separate
 responses for isolation.

---
 control/HTTP.php           | 10 ++++++++--
 tests/control/HTTPTest.php | 21 ++++++++++++++++++++-
 2 files changed, 28 insertions(+), 3 deletions(-)

diff --git a/control/HTTP.php b/control/HTTP.php
index 7a17d54c4..b1b2b3cae 100644
--- a/control/HTTP.php
+++ b/control/HTTP.php
@@ -448,8 +448,14 @@ class HTTP {
 
 		// Now that we've generated them, either output them or attach them to the SS_HTTPResponse as appropriate
 		foreach($responseHeaders as $k => $v) {
-			if($body) $body->addHeader($k, $v);
-			else if(!headers_sent()) header("$k: $v");
+			if($body) {
+				// Set the header now if it's not already set.
+				if ($body->getHeader($k) === null) {
+					$body->addHeader($k, $v);
+				}
+			} elseif(!headers_sent()) {
+				header("$k: $v");
+			}
 		}
 	}
 
diff --git a/tests/control/HTTPTest.php b/tests/control/HTTPTest.php
index 42d6e2d88..b0bd98703 100644
--- a/tests/control/HTTPTest.php
+++ b/tests/control/HTTPTest.php
@@ -13,18 +13,37 @@ class HTTPTest extends FunctionalTest {
 		$this->assertEmpty($response->getHeader('Cache-Control'));
 
 		HTTP::set_cache_age(30);
-		HTTP::add_cache_headers($response);
 
+		HTTP::add_cache_headers($response);
 		$this->assertNotEmpty($response->getHeader('Cache-Control'));
 
+		// Ensure max-age is zero for development.
 		Config::inst()->update('Director', 'environment_type', 'dev');
+		$response = new SS_HTTPResponse($body, 200);
 		HTTP::add_cache_headers($response);
 		$this->assertContains('max-age=0', $response->getHeader('Cache-Control'));
 
+		// Ensure max-age setting is respected in production.
 		Config::inst()->update('Director', 'environment_type', 'live');
+		$response = new SS_HTTPResponse($body, 200);
 		HTTP::add_cache_headers($response);
 		$this->assertContains('max-age=30', explode(', ', $response->getHeader('Cache-Control')));
 		$this->assertNotContains('max-age=0', $response->getHeader('Cache-Control'));
+
+		// Still "live": Ensure header's aren't overridden if already set (using purposefully different values).
+		$headers = array(
+			'Vary' => '*',
+			'Pragma' => 'no-cache',
+			'Cache-Control' => 'max-age=0, no-cache, no-store',
+		);
+		$response = new SS_HTTPResponse($body, 200);
+		foreach($headers as $name => $value) {
+			$response->addHeader($name, $value);
+		}
+		HTTP::add_cache_headers($response);
+		foreach($headers as $name => $value) {
+			$this->assertEquals($value, $response->getHeader($name));
+		}
 	}
 
 	/**