From 76bf2ab21a2e9d9da2194be4194e55b8f8e06b8b Mon Sep 17 00:00:00 2001 From: Damian Mooyman Date: Tue, 12 Jun 2018 17:17:17 +1200 Subject: [PATCH 01/34] WIP of cache middlware --- _config/requestprocessors.yml | 11 +- src/Control/Controller.php | 3 - src/Control/Director.php | 1 - .../Middleware/CanonicalURLMiddleware.php | 1 - .../Middleware/HTTPCacheControlMiddleware.php | 506 ++++++++++++++++++ src/Control/RSS/RSSFeed.php | 5 +- 6 files changed, 517 insertions(+), 10 deletions(-) create mode 100644 src/Control/Middleware/HTTPCacheControlMiddleware.php diff --git a/_config/requestprocessors.yml b/_config/requestprocessors.yml index 65aa88ef8..056f93e7e 100644 --- a/_config/requestprocessors.yml +++ b/_config/requestprocessors.yml @@ -11,6 +11,7 @@ SilverStripe\Core\Injector\Injector: SessionMiddleware: '%$SilverStripe\Control\Middleware\SessionMiddleware' RequestProcessorMiddleware: '%$SilverStripe\Control\RequestProcessor' FlushMiddleware: '%$SilverStripe\Control\Middleware\FlushMiddleware' + HTTPCacheControleMiddleware: '%$SilverStripe\Control\Middleware\HTTPCacheControlMiddleware' CanonicalURLMiddleware: '%$SilverStripe\Control\Middleware\CanonicalURLMiddleware' SilverStripe\Control\Middleware\AllowedHostsMiddleware: properties: @@ -46,4 +47,12 @@ SilverStripe\Core\Injector\Injector: properties: ForceSSL: false ForceWWW: false - +--- +Name: httpcache-dev +Only: + environment: dev +--- +SilverStripe\Core\Injector\Injector: + SilverStripe\Control\Middleware\HTTPCacheControlMiddleware: + Properties: + Enabled: false diff --git a/src/Control/Controller.php b/src/Control/Controller.php index 2b2932aaa..6b73f00c1 100644 --- a/src/Control/Controller.php +++ b/src/Control/Controller.php @@ -258,9 +258,6 @@ class Controller extends RequestHandler implements TemplateGlobalProvider //deal with content if appropriate ContentNegotiator::process($this->getResponse()); - - //add cache headers - HTTP::add_cache_headers($this->getResponse()); } /** diff --git a/src/Control/Director.php b/src/Control/Director.php index 77a2312a8..79622d213 100644 --- a/src/Control/Director.php +++ b/src/Control/Director.php @@ -932,7 +932,6 @@ class Director implements TemplateGlobalProvider // Redirect to installer $response = new HTTPResponse(); $response->redirect($destURL, 301); - HTTP::add_cache_headers($response); throw new HTTPResponse_Exception($response); } diff --git a/src/Control/Middleware/CanonicalURLMiddleware.php b/src/Control/Middleware/CanonicalURLMiddleware.php index c007f0742..006eda696 100644 --- a/src/Control/Middleware/CanonicalURLMiddleware.php +++ b/src/Control/Middleware/CanonicalURLMiddleware.php @@ -407,7 +407,6 @@ class CanonicalURLMiddleware implements HTTPMiddleware // Force redirect $response = HTTPResponse::create(); $response->redirect($url, $this->getRedirectType()); - HTTP::add_cache_headers($response); return $response; } diff --git a/src/Control/Middleware/HTTPCacheControlMiddleware.php b/src/Control/Middleware/HTTPCacheControlMiddleware.php new file mode 100644 index 000000000..54cda3330 --- /dev/null +++ b/src/Control/Middleware/HTTPCacheControlMiddleware.php @@ -0,0 +1,506 @@ +getResponse(); + } + HTTP::add_cache_headers($response); + return $response; + } + + /** + * List of states, each of which contains a key of standard directives. + * Each directive should either be a numeric value, true to enable, + * or (bool)false or null to disable. + * Top level key states include `disabled`, `private`, `public`, `enabled` + * in descending order of precedence. + * + * This allows directives to be set independently for individual states. + * + * @var array + */ + protected $stateDirectives = [ + self::STATE_DISABLED => [ + 'no-cache' => true, + 'no-store' => true, + 'must-revalidate' => true, + ], + self::STATE_PRIVATE => [ + 'private' => true, + 'must-revalidate' => true, + ], + self::STATE_PUBLIC => [ + 'public' => true, + 'must-revalidate' => true, + ], + self::STATE_ENABLED => [ + 'must-revalidate' => true, + ], + ]; + + /** + * Current state + * + * @var string + */ + protected $state = self::STATE_ENABLED; + + /** + * Forcing level of previous setting; higher number wins + * Combination of consts belo + *w + * @var int + */ + protected $forcingLevel = 0; + + /** + * Forcing level forced, optionally combined with one of the below. + */ + const LEVEL_FORCED = 10; + + /** + * Forcing level caching disabled. Overrides public/private. + */ + const LEVEL_DISABLED = 3; + + /** + * Forcing level private-cached. Overrides public. + */ + const LEVEL_PRIVATE = 2; + + /** + * Forcing level public cached. Lowest priority. + */ + const LEVEL_PUBLIC = 1; + + /** + * Forcing level caching enabled. + */ + const LEVEL_ENABLED = 0; + + /** + * A list of allowed cache directives for HTTPResponses + * + * This doesn't include any experimental directives, + * use the config system to add to these if you want to enable them + * + * @config + * @var array + */ + private static $allowed_directives = array( + 'public', + 'private', + 'no-cache', + 'max-age', + 's-maxage', + 'must-revalidate', + 'proxy-revalidate', + 'no-store', + 'no-transform', + ); + + /** + * Set current state. Should only be invoked internally after processing precedence rules. + * + * @param string $state + * @return $this + */ + protected function setState($state) + { + if (!array_key_exists($state, $this->stateDirectives)) { + throw new InvalidArgumentException("Invalid state {$state}"); + } + $this->state = $state; + return $this; + } + + /** + * Get current state + * + * @return string + */ + public function getState() + { + return $this->state; + } + + /** + * Instruct the cache to apply a change with a given level, optionally + * modifying it with a force flag to increase priority of this action. + * + * If the apply level was successful, the change is made and the internal level + * threshold is incremented. + * + * @param int $level Priority of the given change + * @param bool $force If usercode has requested this action is forced to a higher priority. + * Note: Even if $force is set to true, other higher-priority forced changes can still + * cause a change to be rejected if it is below the required threshold. + * @return bool True if the given change is accepted, and that the internal + * level threshold is updated (if necessary) to the new minimum level. + */ + protected function applyChangeLevel($level, $force) + { + $forcingLevel = $level + ($force ? self::LEVEL_FORCED : 0); + if ($forcingLevel < $this->forcingLevel) { + return false; + } + $this->forcingLevel = $forcingLevel; + return true; + } + + /** + * Low level method for setting directives include any experimental or custom ones added via config. + * You need to specify the state (or states) to apply this directive to. + * Can also remove directives with false + * + * @param array|string $states State(s) to apply this directive to + * @param string $directive + * @param int|string|bool $value Flag to set for this value. Set to false to remove, or true to set. + * String or int value assign a specific value. + * @return $this + */ + public function setStateDirective($states, $directive, $value = true) + { + if ($value === null) { + throw new InvalidArgumentException("Invalid directive value"); + } + // make sure the directive is in the list of allowed directives + $allowedDirectives = $this->config()->get('allowed_directives'); + $directive = strtolower($directive); + if (!in_array($directive, $allowedDirectives)) { + throw new InvalidArgumentException('Directive ' . $directive . ' is not allowed'); + } + foreach ((array)$states as $state) { + if (!array_key_exists($state, $this->stateDirectives)) { + throw new InvalidArgumentException("Invalid state {$state}"); + } + // Set or unset directive + if ($value === false) { + unset($this->stateDirectives[$state][$directive]); + } else { + $this->stateDirectives[$state][$directive] = $value; + } + } + return $this; + } + + /** + * Low level method to set directives from an associative array + * + * @param array|string $states State(s) to apply this directive to + * @param array $directives + * @return $this + */ + public function setStateDirectivesFromArray($states, $directives) + { + foreach ($directives as $directive => $value) { + $this->setStateDirective($states, $directive, $value); + } + return $this; + } + + /** + * Low level method for removing directives + * + * @param array|string $states State(s) to remove this directive from + * @param string $directive + * @return $this + */ + public function removeStateDirective($states, $directive) + { + $this->setStateDirective($states, $directive, false); + return $this; + } + + /** + * Low level method to check if a directive is currently set + * + * @param string $state State(s) to apply this directive to + * @param string $directive + * @return bool + */ + public function hasStateDirective($state, $directive) + { + $directive = strtolower($directive); + return isset($this->stateDirectives[$state][$directive]); + } + + /** + * Low level method to get the value of a directive for a state. + * Returns false if there is no directive. + * True means the flag is set, otherwise the value of the directive. + * + * @param string $state + * @param string $directive + * @return int|string|bool + */ + public function getStateDirective($state, $directive) + { + $directive = strtolower($directive); + if (isset($this->stateDirectives[$state][$directive])) { + return $this->stateDirectives[$state][$directive]; + } + return false; + } + + /** + * The cache should not store anything about the client request or server response. + * Affects all non-disabled states. Use setStateDirective() instead to set for a single state. + * Set the no-store directive (also removes max-age and s-maxage for consistency purposes) + * + * @param bool $noStore + * + * @return $this + */ + public function setNoStore($noStore = true) + { + // Affect all non-disabled states + $applyTo = [self::STATE_ENABLED, self::STATE_PRIVATE, self::STATE_PUBLIC]; + if ($noStore) { + $this->setStateDirective($applyTo, 'no-store'); + $this->removeStateDirective($applyTo, 'max-age'); + $this->removeStateDirective($applyTo, 's-maxage'); + } else { + $this->removeStateDirective($applyTo, 'no-store'); + } + return $this; + } + + /** + * Forces caches to submit the request to the origin server for validation before releasing a cached copy. + * Affects all non-disabled states. Use setStateDirective() instead to set for a single state. + * + * @param bool $noCache + * @return $this + */ + public function setNoCache($noCache = true) + { + // Affect all non-disabled states + $applyTo = [self::STATE_ENABLED, self::STATE_PRIVATE, self::STATE_PUBLIC]; + $this->setStateDirective($applyTo, 'no-cache', $noCache); + return $this; + } + + /** + * Specifies the maximum amount of time (seconds) a resource will be considered fresh. + * This directive is relative to the time of the request. + * Affects all non-disabled states. Use setStateDirective() instead to set for a single state. + * + * @param int $age + * @return $this + */ + public function setMaxAge($age) + { + // Affect all non-disabled states + $applyTo = [self::STATE_ENABLED, self::STATE_PRIVATE, self::STATE_PUBLIC]; + $this->setStateDirective($applyTo, 'max-age', $age); + return $this; + } + + /** + * Overrides max-age or the Expires header, but it only applies to shared caches (e.g., proxies) + * and is ignored by a private cache. + * Affects all non-disabled states. Use setStateDirective() instead to set for a single state. + * + * @param int $age + * @return $this + */ + public function setSharedMaxAge($age) + { + // Affect all non-disabled states + $applyTo = [self::STATE_ENABLED, self::STATE_PRIVATE, self::STATE_PUBLIC]; + $this->setStateDirective($applyTo, 's-maxage', $age); + return $this; + } + + /** + * The cache must verify the status of the stale resources before using it and expired ones should not be used. + * Affects all non-disabled states. Use setStateDirective() instead to set for a single state. + * + * @param bool $mustRevalidate + * @return $this + */ + public function setMustRevalidate($mustRevalidate = true) + { + $applyTo = [self::STATE_ENABLED, self::STATE_PRIVATE, self::STATE_PUBLIC]; + $this->setStateDirective($applyTo, 'must-revalidate', $mustRevalidate); + return $this; + } + + /** + * Simple way to set cache control header to a cacheable state. + * + * The resulting cache-control headers will be chosen from the 'enabled' set of directives. + * + * Does not set `public` directive. Usually, `setMaxAge()` is sufficient. Use `publicCache()` if this is explicitly required. + * See https://developers.google.com/web/fundamentals/performance/optimizing-content-efficiency/http-caching#public_vs_private + * + * @see https://docs.silverstripe.org/en/developer_guides/performance/http_cache_headers/ + * @param bool $force Force the cache to public even if its unforced private or public + * @return $this + */ + public function enableCache($force = false) + { + // Only execute this if its forcing level is high enough + if ($this->applyChangeLevel(self::LEVEL_ENABLED, $force)) { + $this->setState(self::STATE_ENABLED); + } + return $this; + } + + /** + * Simple way to set cache control header to a non-cacheable state. + * Use this method over `privateCache()` if you are unsure about caching details. + * Takes precendence over unforced `enableCache()`, `privateCache()` or `publicCache()` calls. + * + * The resulting cache-control headers will be chosen from the 'disabled' set of directives. + * + * Removes all state and replaces it with `no-cache, no-store, must-revalidate`. Although `no-store` is sufficient + * the others are added under recommendation from Mozilla (https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Cache-Control#Examples) + * + * Does not set `private` directive, use `privateCache()` if this is explicitly required. + * See https://developers.google.com/web/fundamentals/performance/optimizing-content-efficiency/http-caching#public_vs_private + * + * @see https://docs.silverstripe.org/en/developer_guides/performance/http_cache_headers/ + * @param bool $force Force the cache to diabled even if it's forced private or public + * @return $this + */ + public function disableCache($force = false) + { + // Only execute this if its forcing level is high enough + if ($this->applyChangeLevel(self::LEVEL_DISABLED, $force )) { + $this->setState(self::STATE_DISABLED); + } + return $this; + } + + /** + * Advanced way to set cache control header to a non-cacheable state. + * Indicates that the response is intended for a single user and must not be stored by a shared cache. + * A private cache (e.g. Web Browser) may store the response. + * + * The resulting cache-control headers will be chosen from the 'private' set of directives. + * + * @see https://docs.silverstripe.org/en/developer_guides/performance/http_cache_headers/ + * @param bool $force Force the cache to private even if it's forced public + * @return $this + */ + public function privateCache($force = false) + { + // Only execute this if its forcing level is high enough + if ($this->applyChangeLevel(self::LEVEL_PRIVATE, $force)) { + $this->setState(self::STATE_PRIVATE); + } + return $this; + } + + /** + * Advanced way to set cache control header to a cacheable state. + * Indicates that the response may be cached by any cache. (eg: CDNs, Proxies, Web browsers) + * + * The resulting cache-control headers will be chosen from the 'private' set of directives. + * + * @see https://docs.silverstripe.org/en/developer_guides/performance/http_cache_headers/ + * @param bool $force Force the cache to public even if it's private, unless it's been forced private + * @return $this + */ + public function publicCache($force = false) + { + // Only execute this if its forcing level is high enough + if (!$this->applyChangeLevel(self::LEVEL_PUBLIC, $force)) { + $this->setState(self::STATE_PUBLIC); + } + return $this; + } + + /** + * Generate and add the `Cache-Control` header to a response object + * + * @param HTTPResponse $response + * + * @return $this + */ + public function applyToResponse($response) + { + $headers = $this->generateHeaders(); + foreach ($headers as $name => $value) { + $response->addHeader($name, $value); + } + return $this; + } + + /** + * Generate the cache header + * + * @return string + */ + protected function generateCacheHeader() + { + $cacheControl = array(); + foreach ($this->state as $directive => $value) { + if (is_null($value)) { + $cacheControl[] = $directive; + } else { + $cacheControl[] = $directive . '=' . $value; + } + } + return implode(', ', $cacheControl); + } + + /** + * Generate all headers to output + * + * @return array + */ + public function generateHeaders() + { + return array( + 'Cache-Control' => $this->generateCacheHeader(), + ); + } + + /** + * Reset registered http cache control and force a fresh instance to be built + */ + public static function reset() + { + Injector::inst()->unregisterNamedObject(__CLASS__); + } +} diff --git a/src/Control/RSS/RSSFeed.php b/src/Control/RSS/RSSFeed.php index e6806129e..12bc4e876 100644 --- a/src/Control/RSS/RSSFeed.php +++ b/src/Control/RSS/RSSFeed.php @@ -233,10 +233,7 @@ class RSSFeed extends ViewableData 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"); SSViewer::config()->update('source_file_comments', $prevState); return $this->renderWith($this->getTemplates()); From 442db3050cc2e146ae60b057d4536d7b48b922d9 Mon Sep 17 00:00:00 2001 From: Damian Mooyman Date: Tue, 12 Jun 2018 17:52:31 +1200 Subject: [PATCH 02/34] Manual merge up of 3.x changes to HTTP class --- _config/config.yml | 15 +- src/Control/HTTP.php | 320 +++++++++++------- src/Control/Middleware/FlushMiddleware.php | 3 + .../Middleware/HTTPCacheControlMiddleware.php | 57 +++- src/Control/RequestHandler.php | 3 +- 5 files changed, 259 insertions(+), 139 deletions(-) diff --git a/_config/config.yml b/_config/config.yml index 7109ca939..1d2a634fb 100644 --- a/_config/config.yml +++ b/_config/config.yml @@ -3,10 +3,17 @@ Name: coreconfig --- SilverStripe\Control\HTTP: cache_control: - max-age: 0 - must-revalidate: "true" - no-transform: "true" - vary: "Cookie, X-Forwarded-Protocol, X-Forwarded-Proto, User-Agent, Accept" + no-cache: true + no-store: true + must-revalidate: true + vary: "X-Requested-With, X-Forwarded-Protocol" SilverStripe\Core\Manifest\VersionProvider: modules: silverstripe/framework: Framework +--- +Name: httpconfig-dev +Only: + environment: dev +--- +SilverStripe\Control\HTTP: + disable_http_cache: true diff --git a/src/Control/HTTP.php b/src/Control/HTTP.php index 08531c38b..daf5f5cec 100644 --- a/src/Control/HTTP.php +++ b/src/Control/HTTP.php @@ -3,6 +3,8 @@ namespace SilverStripe\Control; use SilverStripe\Assets\File; +use SilverStripe\Control\Middleware\HTTPCacheControlMiddleware; +use SilverStripe\Core\Config\Config; use SilverStripe\Core\Config\Configurable; use SilverStripe\Core\Convert; use InvalidArgumentException; @@ -37,6 +39,37 @@ class HTTP */ private static $cache_ajax_requests = true; + /** + * @config + * @var bool + */ + private static $disable_http_cache = false; + + /** + * Mapping of extension to mime types + * + * @var array + * @config + */ + private static $MimeTypes = array(); + + /** + * List of names to add to the Cache-Control header. + * + * @see HTTPCacheControlMiddleware::__construct() + * @config + * @var array Keys are cache control names, values are boolean flags + */ + private static $cache_control = array(); + + /** + * Vary string; A comma separated list of var header names + * + * @config + * @var string|null + */ + private static $vary = null; + /** * Turns a local system filename into a URL by comparing it to the script filename. * @@ -328,6 +361,7 @@ class HTTP public static function set_cache_age($age) { self::$cache_age = $age; + HTTPCacheControlMiddleware::singleton()->setMaxAge(self::$cache_age); } /** @@ -376,169 +410,175 @@ class HTTP */ public static function add_cache_headers($body = null) { - $cacheAge = self::$cache_age; - // Validate argument if ($body && !($body instanceof HTTPResponse)) { user_error("HTTP::add_cache_headers() must be passed an HTTPResponse object", E_USER_WARNING); $body = null; } - // Development sites have frequently changing templates; this can get stuffed up by the code - // below. - if (Director::isDev()) { - $cacheAge = 0; - } - // The headers have been sent and we don't have an HTTPResponse object to attach things to; no point in // us trying. if (headers_sent() && !$body) { return; } - // Populate $responseHeaders with all the headers that we want to build - $responseHeaders = array(); + // Warn if already assigned cache-control headers + if ($body && $body->getHeader('Cache-Control')) { + trigger_error( + 'Cache-Control header has already been set. ' + . 'Please use HTTPCacheControlMiddleware API to set caching options instead.', + E_USER_WARNING + ); + return; + } - $cacheControlHeaders = HTTP::config()->uninherited('cache_control'); + $config = Config::forClass(__CLASS__); + // Get current cache control state + $cacheControl = HTTPCacheControlMiddleware::singleton(); - // currently using a config setting to cancel this, seems to be so that the CMS caches ajax requests - if (function_exists('apache_request_headers') && static::config()->uninherited('cache_ajax_requests')) { - $requestHeaders = array_change_key_case(apache_request_headers(), CASE_LOWER); - if (isset($requestHeaders['x-requested-with']) - && $requestHeaders['x-requested-with']=='XMLHttpRequest' - ) { - $cacheAge = 0; - } - } + // if http caching is disabled by config, disable it - used on dev environments due to frequently changing + // templates and other data. will be overridden by forced publicCache() or privateCache() calls + if ($config->get('disable_http_cache')) { + $cacheControl->disableCache(); + } - if ($cacheAge > 0) { - $cacheControlHeaders['max-age'] = self::$cache_age; + // Populate $responseHeaders with all the headers that we want to build + $responseHeaders = array(); - // Set empty pragma to avoid PHP's session_cache_limiter adding conflicting caching information, - // defaulting to "nocache" on most PHP configurations (see http://php.net/session_cache_limiter). - // Since it's a deprecated HTTP 1.0 option, all modern HTTP clients and proxies should - // prefer the caching information indicated through the "Cache-Control" header. - $responseHeaders["Pragma"] = ""; + // if no caching ajax requests, disable ajax if is ajax request + if (!$config->get('cache_ajax_requests') && Director::is_ajax()) { + $cacheControl->disableCache(); + } - // To do: User-Agent should only be added in situations where you *are* actually - // varying according to user-agent. - $vary = HTTP::config()->uninherited('vary'); - if ($vary && strlen($vary)) { - $responseHeaders['Vary'] = $vary; - } - } else { - $contentDisposition = null; - if ($body) { - // Grab header for checking. Unfortunately HTTPRequest uses a mistyped variant. - $contentDisposition = $body->getHeader('Content-disposition'); - if (!$contentDisposition) { - $contentDisposition = $body->getHeader('Content-Disposition'); - } - } + // Errors disable cache (unless some errors are cached intentionally by usercode) + if ($body && $body->isError()) { + // Even if publicCache(true) is specfied, errors will be uncachable + $cacheControl->disableCache(true); + } - if ($body && - Director::is_https() && - isset($_SERVER['HTTP_USER_AGENT']) && - strstr($_SERVER['HTTP_USER_AGENT'], 'MSIE')==true && - strstr($contentDisposition, 'attachment;')==true - ) { - // IE6-IE8 have problems saving files when https and no-cache are used - // (http://support.microsoft.com/kb/323308) - // Note: this is also fixable by ticking "Do not save encrypted pages to disk" in advanced options. - $cacheControlHeaders['max-age'] = 3; + // split the current vary header into it's parts and merge it with the config settings + // to create a list of unique vary values + $configVary = $config->get('vary'); + $bodyVary = $body ? $body->getHeader('Vary') : ''; + $vary = self::combineVary($configVary, $bodyVary); + if ($vary) { + $responseHeaders['Vary'] = $vary; + } - // Set empty pragma to avoid PHP's session_cache_limiter adding conflicting caching information, - // defaulting to "nocache" on most PHP configurations (see http://php.net/session_cache_limiter). - // Since it's a deprecated HTTP 1.0 option, all modern HTTP clients and proxies should - // prefer the caching information indicated through the "Cache-Control" header. - $responseHeaders["Pragma"] = ""; - } else { - $cacheControlHeaders['no-cache'] = "true"; - $cacheControlHeaders['no-store'] = "true"; - } - } + // deal with IE6-IE8 problems with https and no-cache + $contentDisposition = null; + if($body) { + // Grab header for checking. Unfortunately HTTPRequest uses a mistyped variant. + $contentDisposition = $body->getHeader('Content-Disposition'); + } - foreach ($cacheControlHeaders as $header => $value) { - if (is_null($value)) { - unset($cacheControlHeaders[$header]); - } elseif ((is_bool($value) && $value) || $value === "true") { - $cacheControlHeaders[$header] = $header; - } else { - $cacheControlHeaders[$header] = $header . "=" . $value; - } - } + if( + $body && + Director::is_https() && + isset($_SERVER['HTTP_USER_AGENT']) && + strstr($_SERVER['HTTP_USER_AGENT'], 'MSIE') == true && + strstr($contentDisposition, 'attachment;') == true && + ($cacheControl->hasDirective('no-cache') || $cacheControl->hasDirective('no-store')) + ) { + // IE6-IE8 have problems saving files when https and no-cache/no-store are used + // (http://support.microsoft.com/kb/323308) + // Note: this is also fixable by ticking "Do not save encrypted pages to disk" in advanced options. + $cacheControl->privateCache(true); + } - $responseHeaders['Cache-Control'] = implode(', ', $cacheControlHeaders); - unset($cacheControlHeaders, $header, $value); + if (self::$modification_date) { + $responseHeaders["Last-Modified"] = self::gmt_date(self::$modification_date); + } - if (self::$modification_date && $cacheAge > 0) { - $responseHeaders["Last-Modified"] = self::gmt_date(self::$modification_date); + // if we can store the cache responses we should generate and send etags + if (!$cacheControl->hasDirective('no-store')) { + // Chrome ignores Varies when redirecting back (http://code.google.com/p/chromium/issues/detail?id=79758) + // which means that if you log out, you get redirected back to a page which Chrome then checks against + // last-modified (which passes, getting a 304) + // when it shouldn't be trying to use that page at all because it's the "logged in" version. + // By also using and etag that includes both the modification date and all the varies + // values which we also check against we can catch this and not return a 304 + $etag = self::generateETag($body); + if ($etag) { + $responseHeaders['ETag'] = $etag; - // Chrome ignores Varies when redirecting back (http://code.google.com/p/chromium/issues/detail?id=79758) - // which means that if you log out, you get redirected back to a page which Chrome then checks against - // last-modified (which passes, getting a 304) - // when it shouldn't be trying to use that page at all because it's the "logged in" version. - // By also using and etag that includes both the modification date and all the varies - // values which we also check against we can catch this and not return a 304 - $etagParts = array(self::$modification_date, serialize($_COOKIE)); - $etagParts[] = Director::is_https() ? 'https' : 'http'; - if (isset($_SERVER['HTTP_USER_AGENT'])) { - $etagParts[] = $_SERVER['HTTP_USER_AGENT']; - } - if (isset($_SERVER['HTTP_ACCEPT'])) { - $etagParts[] = $_SERVER['HTTP_ACCEPT']; - } + // 304 response detection + if (isset($_SERVER['HTTP_IF_NONE_MATCH'])) { + // As above, only 304 if the last request had all the same varies values + // (or the etag isn't passed as part of the request - but with chrome it always is) + $matchesEtag = $_SERVER['HTTP_IF_NONE_MATCH'] == $etag; - $etag = sha1(implode(':', $etagParts)); - $responseHeaders["ETag"] = $etag; + if ($matchesEtag) { + if ($body) { + $body->setStatusCode(304); + $body->setBody(''); + } else { + // this is wrong, we need to send the same vary headers and so on + header('HTTP/1.0 304 Not Modified'); + die(); + } + } + } + } + } - // 304 response detection - if (isset($_SERVER['HTTP_IF_MODIFIED_SINCE'])) { - $ifModifiedSince = strtotime(stripslashes($_SERVER['HTTP_IF_MODIFIED_SINCE'])); + if ($cacheControl->hasDirective('max-age')) { + $expires = time() + $cacheControl->getDirective('max-age'); + $responseHeaders["Expires"] = self::gmt_date($expires); + } - // As above, only 304 if the last request had all the same varies values - // (or the etag isn't passed as part of the request - but with chrome it always is) - $matchesEtag = !isset($_SERVER['HTTP_IF_NONE_MATCH']) || $_SERVER['HTTP_IF_NONE_MATCH'] == $etag; + // etag needs to be a quoted string according to HTTP spec + if (!empty($responseHeaders['ETag']) && 0 !== strpos($responseHeaders['ETag'], '"')) { + $responseHeaders['ETag'] = sprintf('"%s"', $responseHeaders['ETag']); + } - if ($ifModifiedSince >= self::$modification_date && $matchesEtag) { - if ($body) { - $body->setStatusCode(304); - $body->setBody(''); - } else { - header('HTTP/1.0 304 Not Modified'); - die(); - } - } - } + // Merge with cache control headers + $responseHeaders = array_merge($responseHeaders, $cacheControl->generateHeaders()); - $expires = time() + $cacheAge; - $responseHeaders["Expires"] = self::gmt_date($expires); - } - - if (self::$etag) { - $responseHeaders['ETag'] = self::$etag; - } - - // etag needs to be a quoted string according to HTTP spec - if (!empty($responseHeaders['ETag']) && 0 !== strpos($responseHeaders['ETag'], '"')) { - $responseHeaders['ETag'] = sprintf('"%s"', $responseHeaders['ETag']); - } - - // Now that we've generated them, either output them or attach them to the HTTPResponse as appropriate - foreach ($responseHeaders as $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"); - } - } + // 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) { + // 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"); + } + } } + + /** + * @param HTTPResponse|string $response + * + * @return string|false + */ + protected static function generateETag($response) + { + // Explicit etag + if (self::$etag) { + return self::$etag; + } + + // Existing e-tag + if ($response instanceof HTTPResponse && $response->getHeader('ETag')) { + return $response->getHeader('ETag'); + } + + // Generate etag from body + $body = $response instanceof HTTPResponse + ? $response->getBody() + : $response; + if ($body) { + return sprintf('"%s"', md5($response)); + } + return false; + } + + /** * Return an {@link http://www.faqs.org/rfcs/rfc2822 RFC 2822} date in the GMT timezone (a timestamp * is always in GMT: the number of seconds since January 1 1970 00:00:00 GMT) @@ -561,4 +601,22 @@ class HTTP { return self::$cache_age; } + + /** + * Combine vary strings + * + * @param string $vary,... Each vary as a separate arg + * @return string + */ + protected static function combineVary($vary) + { + $varies = array(); + foreach (func_get_args() as $arg) { + $argVaries = array_filter(preg_split("/\s*,\s*/", trim($arg))); + if ($argVaries) { + $varies = array_merge($varies, $argVaries); + } + } + return implode(', ', array_unique($varies)); + } } diff --git a/src/Control/Middleware/FlushMiddleware.php b/src/Control/Middleware/FlushMiddleware.php index 0d8f8b482..fbe37f2a9 100644 --- a/src/Control/Middleware/FlushMiddleware.php +++ b/src/Control/Middleware/FlushMiddleware.php @@ -17,6 +17,9 @@ class FlushMiddleware implements HTTPMiddleware public function process(HTTPRequest $request, callable $delegate) { if (array_key_exists('flush', $request->getVars())) { + // Disable cache when flushing + HTTPCacheControlMiddleware::singleton()->disableCache(true); + foreach (ClassInfo::implementorsOf(Flushable::class) as $class) { /** @var Flushable|string $class */ $class::flush(); diff --git a/src/Control/Middleware/HTTPCacheControlMiddleware.php b/src/Control/Middleware/HTTPCacheControlMiddleware.php index 54cda3330..2f92f4a01 100644 --- a/src/Control/Middleware/HTTPCacheControlMiddleware.php +++ b/src/Control/Middleware/HTTPCacheControlMiddleware.php @@ -41,6 +41,14 @@ class HTTPCacheControlMiddleware implements HTTPMiddleware, Resettable } catch (HTTPResponse_Exception $ex) { $response = $ex->getResponse(); } + + // If sessions exist we assume that the responses should not be cached by CDNs / proxies as we are + // likely to be supplying information relevant to the current user only + if ($request->getSession()->getAll()) { + // Don't force in case user code chooses to opt in to public caching + $this->privateCache(); + } + HTTP::add_cache_headers($response); return $response; } @@ -262,6 +270,17 @@ class HTTPCacheControlMiddleware implements HTTPMiddleware, Resettable return isset($this->stateDirectives[$state][$directive]); } + /** + * Check if the current state has the given directive. + * + * @param string $directive + * @return bool + */ + public function hasDirective($directive) + { + return $this->hasStateDirective($this->getState(), $directive); + } + /** * Low level method to get the value of a directive for a state. * Returns false if there is no directive. @@ -280,6 +299,38 @@ class HTTPCacheControlMiddleware implements HTTPMiddleware, Resettable return false; } + /** + * Get the value of the given directive for the current state + * + * @param string $directive + * @return bool|int|string + */ + public function getDirective($directive) + { + return $this->getStateDirective($this->getState(), $directive); + } + + /** + * Get directives for the given state + * + * @param string $state + * @return array + */ + public function getStateDirectives($state) + { + return $this->stateDirectives[$state]; + } + + /** + * Get all directives for the currently active state + * + * @return array + */ + public function getDirectives() + { + return $this->getStateDirectives($this->getState()); + } + /** * The cache should not store anything about the client request or server response. * Affects all non-disabled states. Use setStateDirective() instead to set for a single state. @@ -473,9 +524,9 @@ class HTTPCacheControlMiddleware implements HTTPMiddleware, Resettable */ protected function generateCacheHeader() { - $cacheControl = array(); - foreach ($this->state as $directive => $value) { - if (is_null($value)) { + $cacheControl = []; + foreach ($this->getDirectives() as $directive => $value) { + if ($value === true) { $cacheControl[] = $directive; } else { $cacheControl[] = $directive . '=' . $value; diff --git a/src/Control/RequestHandler.php b/src/Control/RequestHandler.php index 2958b9d0b..5ab831c2f 100644 --- a/src/Control/RequestHandler.php +++ b/src/Control/RequestHandler.php @@ -6,6 +6,7 @@ use BadMethodCallException; use Exception; use InvalidArgumentException; use ReflectionClass; +use SilverStripe\Control\Middleware\HTTPCacheControlMiddleware; use SilverStripe\Core\ClassInfo; use SilverStripe\Core\Config\Config; use SilverStripe\Dev\Debug; @@ -657,7 +658,7 @@ class RequestHandler extends ViewableData public function redirectBack() { // Don't cache the redirect back ever - HTTP::set_cache_age(0); + HTTPCacheControlMiddleware::singleton()->disableCache(true); // Prefer to redirect to ?BackURL, but fall back to Referer header // As a last resort redirect to base url From bf90af4845e411826904588ea128be9ef745de3a Mon Sep 17 00:00:00 2001 From: Daniel Hensby Date: Tue, 12 Jun 2018 12:50:37 +0100 Subject: [PATCH 03/34] Linting fixes --- src/Control/HTTP.php | 360 +++++---- .../Middleware/HTTPCacheControlMiddleware.php | 682 +++++++++--------- 2 files changed, 520 insertions(+), 522 deletions(-) diff --git a/src/Control/HTTP.php b/src/Control/HTTP.php index daf5f5cec..1227dda52 100644 --- a/src/Control/HTTP.php +++ b/src/Control/HTTP.php @@ -39,36 +39,36 @@ class HTTP */ private static $cache_ajax_requests = true; - /** - * @config - * @var bool - */ - private static $disable_http_cache = false; + /** + * @config + * @var bool + */ + private static $disable_http_cache = false; - /** - * Mapping of extension to mime types - * - * @var array - * @config - */ - private static $MimeTypes = array(); + /** + * Mapping of extension to mime types + * + * @var array + * @config + */ + private static $MimeTypes = []; - /** - * List of names to add to the Cache-Control header. - * - * @see HTTPCacheControlMiddleware::__construct() - * @config - * @var array Keys are cache control names, values are boolean flags - */ - private static $cache_control = array(); + /** + * List of names to add to the Cache-Control header. + * + * @see HTTPCacheControlMiddleware::__construct() + * @config + * @var array Keys are cache control names, values are boolean flags + */ + private static $cache_control = []; - /** - * Vary string; A comma separated list of var header names - * - * @config - * @var string|null - */ - private static $vary = null; + /** + * Vary string; A comma separated list of var header names + * + * @config + * @var string|null + */ + private static $vary = null; /** * Turns a local system filename into a URL by comparing it to the script filename. @@ -146,7 +146,7 @@ class HTTP } // Replace attributes - $attribs = array("src", "background", "a" => "href", "link" => "href", "base" => "href"); + $attribs = ["src", "background", "a" => "href", "link" => "href", "base" => "href"]; $regExps = []; foreach ($attribs as $tag => $attrib) { if (!is_numeric($tag)) { @@ -161,7 +161,7 @@ class HTTP } // Replace css styles // @todo - http://www.css3.info/preview/multiple-backgrounds/ - $styles = array('background-image', 'background', 'list-style-image', 'list-style', 'content'); + $styles = ['background-image', 'background', 'list-style-image', 'list-style', 'content']; foreach ($styles as $style) { $regExps[] = "/($style:[^;]*url *\\(\")([^\"]+)(\"\\))/i"; $regExps[] = "/($style:[^;]*url *\\(')([^']+)('\\))/i"; @@ -222,7 +222,7 @@ class HTTP } // Parse params and add new variable - $params = array(); + $params = []; if (isset($parts['query'])) { parse_str($parts['query'], $params); } @@ -230,7 +230,7 @@ class HTTP // Generate URI segments and formatting $scheme = (isset($parts['scheme'])) ? $parts['scheme'] : 'http'; - $user = (isset($parts['user']) && $parts['user'] != '') ? $parts['user'] : ''; + $user = (isset($parts['user']) && $parts['user'] != '') ? $parts['user'] : ''; if ($user != '') { // format in either user:pass@host.com or user@host.com @@ -242,13 +242,13 @@ class HTTP $path = (isset($parts['path']) && $parts['path'] != '') ? $parts['path'] : ''; // handle URL params which are existing / new - $params = ($params) ? '?' . http_build_query($params, null, $separator) : ''; + $params = ($params) ? '?' . http_build_query($params, null, $separator) : ''; // keep fragments (anchors) intact. - $fragment = (isset($parts['fragment']) && $parts['fragment'] != '') ? '#' . $parts['fragment'] : ''; + $fragment = (isset($parts['fragment']) && $parts['fragment'] != '') ? '#' . $parts['fragment'] : ''; // Recompile URI segments - $newUri = $scheme . '://' . $user . $host . $port . $path . $params . $fragment; + $newUri = $scheme . '://' . $user . $host . $port . $path . $params . $fragment; if ($isRelative) { return Director::makeRelative($newUri); @@ -281,14 +281,14 @@ class HTTP */ public static function findByTagAndAttribute($content, $attributes) { - $regexes = array(); + $regexes = []; foreach ($attributes as $tag => $attribute) { $regexes[] = "/<{$tag} [^>]*$attribute *= *([\"'])(.*?)\\1[^>]*>/i"; $regexes[] = "/<{$tag} [^>]*$attribute *= *([^ \"'>]+)/i"; } - $result = array(); + $result = []; if ($regexes) { foreach ($regexes as $regex) { @@ -308,7 +308,7 @@ class HTTP */ public static function getLinksIn($content) { - return self::findByTagAndAttribute($content, array("a" => "href")); + return self::findByTagAndAttribute($content, ["a" => "href"]); } /** @@ -318,7 +318,7 @@ class HTTP */ public static function getImagesIn($content) { - return self::findByTagAndAttribute($content, array("img" => "src")); + return self::findByTagAndAttribute($content, ["img" => "src"]); } /** @@ -423,148 +423,146 @@ class HTTP } // Warn if already assigned cache-control headers - if ($body && $body->getHeader('Cache-Control')) { - trigger_error( - 'Cache-Control header has already been set. ' - . 'Please use HTTPCacheControlMiddleware API to set caching options instead.', - E_USER_WARNING - ); - return; - } + if ($body && $body->getHeader('Cache-Control')) { + trigger_error( + 'Cache-Control header has already been set. ' + . 'Please use HTTPCacheControlMiddleware API to set caching options instead.', + E_USER_WARNING + ); + return; + } - $config = Config::forClass(__CLASS__); + $config = Config::forClass(__CLASS__); - // Get current cache control state - $cacheControl = HTTPCacheControlMiddleware::singleton(); + // Get current cache control state + $cacheControl = HTTPCacheControlMiddleware::singleton(); - // if http caching is disabled by config, disable it - used on dev environments due to frequently changing - // templates and other data. will be overridden by forced publicCache() or privateCache() calls - if ($config->get('disable_http_cache')) { - $cacheControl->disableCache(); - } + // if http caching is disabled by config, disable it - used on dev environments due to frequently changing + // templates and other data. will be overridden by forced publicCache() or privateCache() calls + if ($config->get('disable_http_cache')) { + $cacheControl->disableCache(); + } - // Populate $responseHeaders with all the headers that we want to build - $responseHeaders = array(); + // Populate $responseHeaders with all the headers that we want to build + $responseHeaders = []; - // if no caching ajax requests, disable ajax if is ajax request - if (!$config->get('cache_ajax_requests') && Director::is_ajax()) { - $cacheControl->disableCache(); - } + // if no caching ajax requests, disable ajax if is ajax request + if (!$config->get('cache_ajax_requests') && Director::is_ajax()) { + $cacheControl->disableCache(); + } - // Errors disable cache (unless some errors are cached intentionally by usercode) - if ($body && $body->isError()) { - // Even if publicCache(true) is specfied, errors will be uncachable - $cacheControl->disableCache(true); - } + // Errors disable cache (unless some errors are cached intentionally by usercode) + if ($body && $body->isError()) { + // Even if publicCache(true) is specfied, errors will be uncachable + $cacheControl->disableCache(true); + } - // split the current vary header into it's parts and merge it with the config settings - // to create a list of unique vary values - $configVary = $config->get('vary'); - $bodyVary = $body ? $body->getHeader('Vary') : ''; - $vary = self::combineVary($configVary, $bodyVary); - if ($vary) { - $responseHeaders['Vary'] = $vary; - } + // split the current vary header into it's parts and merge it with the config settings + // to create a list of unique vary values + $configVary = $config->get('vary'); + $bodyVary = $body ? $body->getHeader('Vary') : ''; + $vary = self::combineVary($configVary, $bodyVary); + if ($vary) { + $responseHeaders['Vary'] = $vary; + } - // deal with IE6-IE8 problems with https and no-cache - $contentDisposition = null; - if($body) { - // Grab header for checking. Unfortunately HTTPRequest uses a mistyped variant. - $contentDisposition = $body->getHeader('Content-Disposition'); - } + // deal with IE6-IE8 problems with https and no-cache + $contentDisposition = null; + if ($body) { + // Grab header for checking. Unfortunately HTTPRequest uses a mistyped variant. + $contentDisposition = $body->getHeader('Content-Disposition'); + } - if( - $body && - Director::is_https() && - isset($_SERVER['HTTP_USER_AGENT']) && - strstr($_SERVER['HTTP_USER_AGENT'], 'MSIE') == true && - strstr($contentDisposition, 'attachment;') == true && - ($cacheControl->hasDirective('no-cache') || $cacheControl->hasDirective('no-store')) - ) { - // IE6-IE8 have problems saving files when https and no-cache/no-store are used - // (http://support.microsoft.com/kb/323308) - // Note: this is also fixable by ticking "Do not save encrypted pages to disk" in advanced options. - $cacheControl->privateCache(true); - } + if ($body && + Director::is_https() && + isset($_SERVER['HTTP_USER_AGENT']) && + strstr($_SERVER['HTTP_USER_AGENT'], 'MSIE') == true && + strstr($contentDisposition, 'attachment;') == true && + ($cacheControl->hasDirective('no-cache') || $cacheControl->hasDirective('no-store')) + ) { + // IE6-IE8 have problems saving files when https and no-cache/no-store are used + // (http://support.microsoft.com/kb/323308) + // Note: this is also fixable by ticking "Do not save encrypted pages to disk" in advanced options. + $cacheControl->privateCache(true); + } - if (self::$modification_date) { - $responseHeaders["Last-Modified"] = self::gmt_date(self::$modification_date); - } + if (self::$modification_date) { + $responseHeaders["Last-Modified"] = self::gmt_date(self::$modification_date); + } - // if we can store the cache responses we should generate and send etags - if (!$cacheControl->hasDirective('no-store')) { - // Chrome ignores Varies when redirecting back (http://code.google.com/p/chromium/issues/detail?id=79758) - // which means that if you log out, you get redirected back to a page which Chrome then checks against - // last-modified (which passes, getting a 304) - // when it shouldn't be trying to use that page at all because it's the "logged in" version. - // By also using and etag that includes both the modification date and all the varies - // values which we also check against we can catch this and not return a 304 - $etag = self::generateETag($body); - if ($etag) { - $responseHeaders['ETag'] = $etag; + // if we can store the cache responses we should generate and send etags + if (!$cacheControl->hasDirective('no-store')) { + // Chrome ignores Varies when redirecting back (http://code.google.com/p/chromium/issues/detail?id=79758) + // which means that if you log out, you get redirected back to a page which Chrome then checks against + // last-modified (which passes, getting a 304) + // when it shouldn't be trying to use that page at all because it's the "logged in" version. + // By also using and etag that includes both the modification date and all the varies + // values which we also check against we can catch this and not return a 304 + $etag = self::generateETag($body); + if ($etag) { + $responseHeaders['ETag'] = $etag; - // 304 response detection - if (isset($_SERVER['HTTP_IF_NONE_MATCH'])) { - // As above, only 304 if the last request had all the same varies values - // (or the etag isn't passed as part of the request - but with chrome it always is) - $matchesEtag = $_SERVER['HTTP_IF_NONE_MATCH'] == $etag; + // 304 response detection + if (isset($_SERVER['HTTP_IF_NONE_MATCH'])) { + // As above, only 304 if the last request had all the same varies values + // (or the etag isn't passed as part of the request - but with chrome it always is) + $matchesEtag = $_SERVER['HTTP_IF_NONE_MATCH'] == $etag; - if ($matchesEtag) { - if ($body) { - $body->setStatusCode(304); - $body->setBody(''); - } else { - // this is wrong, we need to send the same vary headers and so on - header('HTTP/1.0 304 Not Modified'); - die(); - } - } - } - } - } + if ($matchesEtag) { + if ($body) { + $body->setStatusCode(304); + $body->setBody(''); + } else { + // this is wrong, we need to send the same vary headers and so on + header('HTTP/1.0 304 Not Modified'); + die(); + } + } + } + } + } - if ($cacheControl->hasDirective('max-age')) { - $expires = time() + $cacheControl->getDirective('max-age'); - $responseHeaders["Expires"] = self::gmt_date($expires); - } + if ($cacheControl->hasDirective('max-age')) { + $expires = time() + $cacheControl->getDirective('max-age'); + $responseHeaders["Expires"] = self::gmt_date($expires); + } - // etag needs to be a quoted string according to HTTP spec - if (!empty($responseHeaders['ETag']) && 0 !== strpos($responseHeaders['ETag'], '"')) { - $responseHeaders['ETag'] = sprintf('"%s"', $responseHeaders['ETag']); - } + // etag needs to be a quoted string according to HTTP spec + if (!empty($responseHeaders['ETag']) && 0 !== strpos($responseHeaders['ETag'], '"')) { + $responseHeaders['ETag'] = sprintf('"%s"', $responseHeaders['ETag']); + } - // Merge with cache control headers - $responseHeaders = array_merge($responseHeaders, $cacheControl->generateHeaders()); + // Merge with cache control headers + $responseHeaders = array_merge($responseHeaders, $cacheControl->generateHeaders()); - // 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) { - // 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"); - } - } + // 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) { + // 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"); + } + } } + /** + * @param HTTPResponse|string $response + * + * @return string|false + */ + protected static function generateETag($response) + { + // Explicit etag + if (self::$etag) { + return self::$etag; + } - /** - * @param HTTPResponse|string $response - * - * @return string|false - */ - protected static function generateETag($response) - { - // Explicit etag - if (self::$etag) { - return self::$etag; - } - - // Existing e-tag - if ($response instanceof HTTPResponse && $response->getHeader('ETag')) { + // Existing e-tag + if ($response instanceof HTTPResponse && $response->getHeader('ETag')) { return $response->getHeader('ETag'); } @@ -572,11 +570,11 @@ class HTTP $body = $response instanceof HTTPResponse ? $response->getBody() : $response; - if ($body) { - return sprintf('"%s"', md5($response)); - } - return false; - } + if ($body) { + return sprintf('"%s"', md5($response)); + } + return false; + } /** @@ -602,21 +600,21 @@ class HTTP return self::$cache_age; } - /** - * Combine vary strings - * - * @param string $vary,... Each vary as a separate arg - * @return string - */ - protected static function combineVary($vary) - { - $varies = array(); - foreach (func_get_args() as $arg) { - $argVaries = array_filter(preg_split("/\s*,\s*/", trim($arg))); - if ($argVaries) { - $varies = array_merge($varies, $argVaries); - } - } - return implode(', ', array_unique($varies)); - } + /** + * Combine vary strings + * + * @param string $vary,... Each vary as a separate arg + * @return string + */ + protected static function combineVary($vary) + { + $varies = []; + foreach (func_get_args() as $arg) { + $argVaries = array_filter(preg_split("/\s*,\s*/", trim($arg))); + if ($argVaries) { + $varies = array_merge($varies, $argVaries); + } + } + return implode(', ', array_unique($varies)); + } } diff --git a/src/Control/Middleware/HTTPCacheControlMiddleware.php b/src/Control/Middleware/HTTPCacheControlMiddleware.php index 2f92f4a01..9db482432 100644 --- a/src/Control/Middleware/HTTPCacheControlMiddleware.php +++ b/src/Control/Middleware/HTTPCacheControlMiddleware.php @@ -42,33 +42,33 @@ class HTTPCacheControlMiddleware implements HTTPMiddleware, Resettable $response = $ex->getResponse(); } - // If sessions exist we assume that the responses should not be cached by CDNs / proxies as we are - // likely to be supplying information relevant to the current user only - if ($request->getSession()->getAll()) { - // Don't force in case user code chooses to opt in to public caching - $this->privateCache(); - } + // If sessions exist we assume that the responses should not be cached by CDNs / proxies as we are + // likely to be supplying information relevant to the current user only + if ($request->getSession()->getAll()) { + // Don't force in case user code chooses to opt in to public caching + $this->privateCache(); + } HTTP::add_cache_headers($response); return $response; } - /** - * List of states, each of which contains a key of standard directives. + /** + * List of states, each of which contains a key of standard directives. * Each directive should either be a numeric value, true to enable, * or (bool)false or null to disable. * Top level key states include `disabled`, `private`, `public`, `enabled` * in descending order of precedence. * * This allows directives to be set independently for individual states. - * - * @var array - */ - protected $stateDirectives = [ - self::STATE_DISABLED => [ - 'no-cache' => true, - 'no-store' => true, - 'must-revalidate' => true, + * + * @var array + */ + protected $stateDirectives = [ + self::STATE_DISABLED => [ + 'no-cache' => true, + 'no-store' => true, + 'must-revalidate' => true, ], self::STATE_PRIVATE => [ 'private' => true, @@ -88,61 +88,61 @@ class HTTPCacheControlMiddleware implements HTTPMiddleware, Resettable * * @var string */ - protected $state = self::STATE_ENABLED; + protected $state = self::STATE_ENABLED; - /** - * Forcing level of previous setting; higher number wins - * Combination of consts belo - *w - * @var int - */ - protected $forcingLevel = 0; + /** + * Forcing level of previous setting; higher number wins + * Combination of consts belo + *w + * @var int + */ + protected $forcingLevel = 0; - /** - * Forcing level forced, optionally combined with one of the below. - */ - const LEVEL_FORCED = 10; + /** + * Forcing level forced, optionally combined with one of the below. + */ + const LEVEL_FORCED = 10; - /** - * Forcing level caching disabled. Overrides public/private. - */ - const LEVEL_DISABLED = 3; + /** + * Forcing level caching disabled. Overrides public/private. + */ + const LEVEL_DISABLED = 3; - /** - * Forcing level private-cached. Overrides public. - */ - const LEVEL_PRIVATE = 2; + /** + * Forcing level private-cached. Overrides public. + */ + const LEVEL_PRIVATE = 2; - /** - * Forcing level public cached. Lowest priority. - */ - const LEVEL_PUBLIC = 1; + /** + * Forcing level public cached. Lowest priority. + */ + const LEVEL_PUBLIC = 1; - /** - * Forcing level caching enabled. - */ - const LEVEL_ENABLED = 0; + /** + * Forcing level caching enabled. + */ + const LEVEL_ENABLED = 0; - /** - * A list of allowed cache directives for HTTPResponses - * - * This doesn't include any experimental directives, - * use the config system to add to these if you want to enable them - * - * @config - * @var array - */ - private static $allowed_directives = array( - 'public', - 'private', - 'no-cache', - 'max-age', - 's-maxage', - 'must-revalidate', - 'proxy-revalidate', - 'no-store', - 'no-transform', - ); + /** + * A list of allowed cache directives for HTTPResponses + * + * This doesn't include any experimental directives, + * use the config system to add to these if you want to enable them + * + * @config + * @var array + */ + private static $allowed_directives = [ + 'public', + 'private', + 'no-cache', + 'max-age', + 's-maxage', + 'must-revalidate', + 'proxy-revalidate', + 'no-store', + 'no-transform', + ]; /** * Set current state. Should only be invoked internally after processing precedence rules. @@ -150,7 +150,7 @@ class HTTPCacheControlMiddleware implements HTTPMiddleware, Resettable * @param string $state * @return $this */ - protected function setState($state) + protected function setState($state) { if (!array_key_exists($state, $this->stateDirectives)) { throw new InvalidArgumentException("Invalid state {$state}"); @@ -169,74 +169,74 @@ class HTTPCacheControlMiddleware implements HTTPMiddleware, Resettable return $this->state; } - /** - * Instruct the cache to apply a change with a given level, optionally - * modifying it with a force flag to increase priority of this action. - * - * If the apply level was successful, the change is made and the internal level - * threshold is incremented. - * - * @param int $level Priority of the given change - * @param bool $force If usercode has requested this action is forced to a higher priority. - * Note: Even if $force is set to true, other higher-priority forced changes can still - * cause a change to be rejected if it is below the required threshold. - * @return bool True if the given change is accepted, and that the internal - * level threshold is updated (if necessary) to the new minimum level. - */ - protected function applyChangeLevel($level, $force) - { - $forcingLevel = $level + ($force ? self::LEVEL_FORCED : 0); - if ($forcingLevel < $this->forcingLevel) { - return false; - } - $this->forcingLevel = $forcingLevel; - return true; - } + /** + * Instruct the cache to apply a change with a given level, optionally + * modifying it with a force flag to increase priority of this action. + * + * If the apply level was successful, the change is made and the internal level + * threshold is incremented. + * + * @param int $level Priority of the given change + * @param bool $force If usercode has requested this action is forced to a higher priority. + * Note: Even if $force is set to true, other higher-priority forced changes can still + * cause a change to be rejected if it is below the required threshold. + * @return bool True if the given change is accepted, and that the internal + * level threshold is updated (if necessary) to the new minimum level. + */ + protected function applyChangeLevel($level, $force) + { + $forcingLevel = $level + ($force ? self::LEVEL_FORCED : 0); + if ($forcingLevel < $this->forcingLevel) { + return false; + } + $this->forcingLevel = $forcingLevel; + return true; + } - /** - * Low level method for setting directives include any experimental or custom ones added via config. + /** + * Low level method for setting directives include any experimental or custom ones added via config. * You need to specify the state (or states) to apply this directive to. * Can also remove directives with false - * + * * @param array|string $states State(s) to apply this directive to - * @param string $directive - * @param int|string|bool $value Flag to set for this value. Set to false to remove, or true to set. + * @param string $directive + * @param int|string|bool $value Flag to set for this value. Set to false to remove, or true to set. * String or int value assign a specific value. - * @return $this - */ - public function setStateDirective($states, $directive, $value = true) - { - if ($value === null) { - throw new InvalidArgumentException("Invalid directive value"); + * @return $this + */ + public function setStateDirective($states, $directive, $value = true) + { + if ($value === null) { + throw new InvalidArgumentException("Invalid directive value"); } - // make sure the directive is in the list of allowed directives - $allowedDirectives = $this->config()->get('allowed_directives'); - $directive = strtolower($directive); - if (!in_array($directive, $allowedDirectives)) { + // make sure the directive is in the list of allowed directives + $allowedDirectives = $this->config()->get('allowed_directives'); + $directive = strtolower($directive); + if (!in_array($directive, $allowedDirectives)) { throw new InvalidArgumentException('Directive ' . $directive . ' is not allowed'); } foreach ((array)$states as $state) { - if (!array_key_exists($state, $this->stateDirectives)) { + if (!array_key_exists($state, $this->stateDirectives)) { throw new InvalidArgumentException("Invalid state {$state}"); } // Set or unset directive if ($value === false) { - unset($this->stateDirectives[$state][$directive]); + unset($this->stateDirectives[$state][$directive]); } else { $this->stateDirectives[$state][$directive] = $value; } } - return $this; - } + return $this; + } /** - * Low level method to set directives from an associative array - * + * Low level method to set directives from an associative array + * * @param array|string $states State(s) to apply this directive to - * @param array $directives - * @return $this - */ - public function setStateDirectivesFromArray($states, $directives) + * @param array $directives + * @return $this + */ + public function setStateDirectivesFromArray($states, $directives) { foreach ($directives as $directive => $value) { $this->setStateDirective($states, $directive, $value); @@ -244,31 +244,31 @@ class HTTPCacheControlMiddleware implements HTTPMiddleware, Resettable return $this; } - /** - * Low level method for removing directives - * + /** + * Low level method for removing directives + * * @param array|string $states State(s) to remove this directive from - * @param string $directive - * @return $this - */ - public function removeStateDirective($states, $directive) + * @param string $directive + * @return $this + */ + public function removeStateDirective($states, $directive) { $this->setStateDirective($states, $directive, false); return $this; } - /** - * Low level method to check if a directive is currently set - * + /** + * Low level method to check if a directive is currently set + * * @param string $state State(s) to apply this directive to - * @param string $directive - * @return bool - */ - public function hasStateDirective($state, $directive) - { + * @param string $directive + * @return bool + */ + public function hasStateDirective($state, $directive) + { $directive = strtolower($directive); return isset($this->stateDirectives[$state][$directive]); - } + } /** * Check if the current state has the given directive. @@ -276,28 +276,28 @@ class HTTPCacheControlMiddleware implements HTTPMiddleware, Resettable * @param string $directive * @return bool */ - public function hasDirective($directive) + public function hasDirective($directive) { return $this->hasStateDirective($this->getState(), $directive); } - /** - * Low level method to get the value of a directive for a state. + /** + * Low level method to get the value of a directive for a state. * Returns false if there is no directive. * True means the flag is set, otherwise the value of the directive. - * + * * @param string $state - * @param string $directive - * @return int|string|bool - */ - public function getStateDirective($state, $directive) - { - $directive = strtolower($directive); + * @param string $directive + * @return int|string|bool + */ + public function getStateDirective($state, $directive) + { + $directive = strtolower($directive); if (isset($this->stateDirectives[$state][$directive])) { return $this->stateDirectives[$state][$directive]; } return false; - } + } /** * Get the value of the given directive for the current state @@ -307,7 +307,7 @@ class HTTPCacheControlMiddleware implements HTTPMiddleware, Resettable */ public function getDirective($directive) { - return $this->getStateDirective($this->getState(), $directive); + return $this->getStateDirective($this->getState(), $directive); } /** @@ -331,227 +331,227 @@ class HTTPCacheControlMiddleware implements HTTPMiddleware, Resettable return $this->getStateDirectives($this->getState()); } - /** - * The cache should not store anything about the client request or server response. - * Affects all non-disabled states. Use setStateDirective() instead to set for a single state. - * Set the no-store directive (also removes max-age and s-maxage for consistency purposes) - * - * @param bool $noStore - * - * @return $this - */ - public function setNoStore($noStore = true) - { - // Affect all non-disabled states + /** + * The cache should not store anything about the client request or server response. + * Affects all non-disabled states. Use setStateDirective() instead to set for a single state. + * Set the no-store directive (also removes max-age and s-maxage for consistency purposes) + * + * @param bool $noStore + * + * @return $this + */ + public function setNoStore($noStore = true) + { + // Affect all non-disabled states $applyTo = [self::STATE_ENABLED, self::STATE_PRIVATE, self::STATE_PUBLIC]; if ($noStore) { - $this->setStateDirective($applyTo, 'no-store'); - $this->removeStateDirective($applyTo, 'max-age'); - $this->removeStateDirective($applyTo, 's-maxage'); - } else { - $this->removeStateDirective($applyTo, 'no-store'); - } - return $this; - } + $this->setStateDirective($applyTo, 'no-store'); + $this->removeStateDirective($applyTo, 'max-age'); + $this->removeStateDirective($applyTo, 's-maxage'); + } else { + $this->removeStateDirective($applyTo, 'no-store'); + } + return $this; + } - /** - * Forces caches to submit the request to the origin server for validation before releasing a cached copy. + /** + * Forces caches to submit the request to the origin server for validation before releasing a cached copy. * Affects all non-disabled states. Use setStateDirective() instead to set for a single state. - * - * @param bool $noCache - * @return $this - */ - public function setNoCache($noCache = true) - { - // Affect all non-disabled states - $applyTo = [self::STATE_ENABLED, self::STATE_PRIVATE, self::STATE_PUBLIC]; - $this->setStateDirective($applyTo, 'no-cache', $noCache); - return $this; - } + * + * @param bool $noCache + * @return $this + */ + public function setNoCache($noCache = true) + { + // Affect all non-disabled states + $applyTo = [self::STATE_ENABLED, self::STATE_PRIVATE, self::STATE_PUBLIC]; + $this->setStateDirective($applyTo, 'no-cache', $noCache); + return $this; + } - /** - * Specifies the maximum amount of time (seconds) a resource will be considered fresh. - * This directive is relative to the time of the request. + /** + * Specifies the maximum amount of time (seconds) a resource will be considered fresh. + * This directive is relative to the time of the request. * Affects all non-disabled states. Use setStateDirective() instead to set for a single state. - * - * @param int $age - * @return $this - */ - public function setMaxAge($age) - { - // Affect all non-disabled states - $applyTo = [self::STATE_ENABLED, self::STATE_PRIVATE, self::STATE_PUBLIC]; - $this->setStateDirective($applyTo, 'max-age', $age); - return $this; - } - - /** - * Overrides max-age or the Expires header, but it only applies to shared caches (e.g., proxies) - * and is ignored by a private cache. - * Affects all non-disabled states. Use setStateDirective() instead to set for a single state. * - * @param int $age - * @return $this - */ - public function setSharedMaxAge($age) - { - // Affect all non-disabled states - $applyTo = [self::STATE_ENABLED, self::STATE_PRIVATE, self::STATE_PUBLIC]; - $this->setStateDirective($applyTo, 's-maxage', $age); - return $this; - } + * @param int $age + * @return $this + */ + public function setMaxAge($age) + { + // Affect all non-disabled states + $applyTo = [self::STATE_ENABLED, self::STATE_PRIVATE, self::STATE_PUBLIC]; + $this->setStateDirective($applyTo, 'max-age', $age); + return $this; + } - /** - * The cache must verify the status of the stale resources before using it and expired ones should not be used. - * Affects all non-disabled states. Use setStateDirective() instead to set for a single state. + /** + * Overrides max-age or the Expires header, but it only applies to shared caches (e.g., proxies) + * and is ignored by a private cache. + * Affects all non-disabled states. Use setStateDirective() instead to set for a single state. * - * @param bool $mustRevalidate - * @return $this - */ - public function setMustRevalidate($mustRevalidate = true) - { - $applyTo = [self::STATE_ENABLED, self::STATE_PRIVATE, self::STATE_PUBLIC]; - $this->setStateDirective($applyTo, 'must-revalidate', $mustRevalidate); - return $this; - } + * @param int $age + * @return $this + */ + public function setSharedMaxAge($age) + { + // Affect all non-disabled states + $applyTo = [self::STATE_ENABLED, self::STATE_PRIVATE, self::STATE_PUBLIC]; + $this->setStateDirective($applyTo, 's-maxage', $age); + return $this; + } - /** - * Simple way to set cache control header to a cacheable state. + /** + * The cache must verify the status of the stale resources before using it and expired ones should not be used. + * Affects all non-disabled states. Use setStateDirective() instead to set for a single state. + * + * @param bool $mustRevalidate + * @return $this + */ + public function setMustRevalidate($mustRevalidate = true) + { + $applyTo = [self::STATE_ENABLED, self::STATE_PRIVATE, self::STATE_PUBLIC]; + $this->setStateDirective($applyTo, 'must-revalidate', $mustRevalidate); + return $this; + } + + /** + * Simple way to set cache control header to a cacheable state. * * The resulting cache-control headers will be chosen from the 'enabled' set of directives. - * - * Does not set `public` directive. Usually, `setMaxAge()` is sufficient. Use `publicCache()` if this is explicitly required. - * See https://developers.google.com/web/fundamentals/performance/optimizing-content-efficiency/http-caching#public_vs_private - * - * @see https://docs.silverstripe.org/en/developer_guides/performance/http_cache_headers/ - * @param bool $force Force the cache to public even if its unforced private or public - * @return $this - */ - public function enableCache($force = false) - { - // Only execute this if its forcing level is high enough - if ($this->applyChangeLevel(self::LEVEL_ENABLED, $force)) { + * + * Does not set `public` directive. Usually, `setMaxAge()` is sufficient. Use `publicCache()` if this is explicitly required. + * See https://developers.google.com/web/fundamentals/performance/optimizing-content-efficiency/http-caching#public_vs_private + * + * @see https://docs.silverstripe.org/en/developer_guides/performance/http_cache_headers/ + * @param bool $force Force the cache to public even if its unforced private or public + * @return $this + */ + public function enableCache($force = false) + { + // Only execute this if its forcing level is high enough + if ($this->applyChangeLevel(self::LEVEL_ENABLED, $force)) { $this->setState(self::STATE_ENABLED); } return $this; - } + } - /** + /** * Simple way to set cache control header to a non-cacheable state. - * Use this method over `privateCache()` if you are unsure about caching details. + * Use this method over `privateCache()` if you are unsure about caching details. * Takes precendence over unforced `enableCache()`, `privateCache()` or `publicCache()` calls. * * The resulting cache-control headers will be chosen from the 'disabled' set of directives. - * - * Removes all state and replaces it with `no-cache, no-store, must-revalidate`. Although `no-store` is sufficient - * the others are added under recommendation from Mozilla (https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Cache-Control#Examples) - * - * Does not set `private` directive, use `privateCache()` if this is explicitly required. - * See https://developers.google.com/web/fundamentals/performance/optimizing-content-efficiency/http-caching#public_vs_private - * - * @see https://docs.silverstripe.org/en/developer_guides/performance/http_cache_headers/ - * @param bool $force Force the cache to diabled even if it's forced private or public - * @return $this - */ - public function disableCache($force = false) - { - // Only execute this if its forcing level is high enough - if ($this->applyChangeLevel(self::LEVEL_DISABLED, $force )) { - $this->setState(self::STATE_DISABLED); - } - return $this; - } - - /** - * Advanced way to set cache control header to a non-cacheable state. - * Indicates that the response is intended for a single user and must not be stored by a shared cache. - * A private cache (e.g. Web Browser) may store the response. * - * The resulting cache-control headers will be chosen from the 'private' set of directives. - * - * @see https://docs.silverstripe.org/en/developer_guides/performance/http_cache_headers/ - * @param bool $force Force the cache to private even if it's forced public - * @return $this - */ - public function privateCache($force = false) - { - // Only execute this if its forcing level is high enough - if ($this->applyChangeLevel(self::LEVEL_PRIVATE, $force)) { - $this->setState(self::STATE_PRIVATE); - } - return $this; - } - - /** - * Advanced way to set cache control header to a cacheable state. - * Indicates that the response may be cached by any cache. (eg: CDNs, Proxies, Web browsers) + * Removes all state and replaces it with `no-cache, no-store, must-revalidate`. Although `no-store` is sufficient + * the others are added under recommendation from Mozilla (https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Cache-Control#Examples) * - * The resulting cache-control headers will be chosen from the 'private' set of directives. - * - * @see https://docs.silverstripe.org/en/developer_guides/performance/http_cache_headers/ - * @param bool $force Force the cache to public even if it's private, unless it's been forced private - * @return $this - */ - public function publicCache($force = false) - { - // Only execute this if its forcing level is high enough - if (!$this->applyChangeLevel(self::LEVEL_PUBLIC, $force)) { - $this->setState(self::STATE_PUBLIC); - } - return $this; - } - - /** - * Generate and add the `Cache-Control` header to a response object - * - * @param HTTPResponse $response - * - * @return $this - */ - public function applyToResponse($response) - { - $headers = $this->generateHeaders(); - foreach ($headers as $name => $value) { - $response->addHeader($name, $value); - } - return $this; - } - - /** - * Generate the cache header - * - * @return string - */ - protected function generateCacheHeader() - { - $cacheControl = []; - foreach ($this->getDirectives() as $directive => $value) { - if ($value === true) { - $cacheControl[] = $directive; - } else { - $cacheControl[] = $directive . '=' . $value; - } - } - return implode(', ', $cacheControl); - } - - /** - * Generate all headers to output - * - * @return array - */ - public function generateHeaders() - { - return array( - 'Cache-Control' => $this->generateCacheHeader(), - ); - } - - /** - * Reset registered http cache control and force a fresh instance to be built - */ - public static function reset() + * Does not set `private` directive, use `privateCache()` if this is explicitly required. + * See https://developers.google.com/web/fundamentals/performance/optimizing-content-efficiency/http-caching#public_vs_private + * + * @see https://docs.silverstripe.org/en/developer_guides/performance/http_cache_headers/ + * @param bool $force Force the cache to diabled even if it's forced private or public + * @return $this + */ + public function disableCache($force = false) { - Injector::inst()->unregisterNamedObject(__CLASS__); - } + // Only execute this if its forcing level is high enough + if ($this->applyChangeLevel(self::LEVEL_DISABLED, $force)) { + $this->setState(self::STATE_DISABLED); + } + return $this; + } + + /** + * Advanced way to set cache control header to a non-cacheable state. + * Indicates that the response is intended for a single user and must not be stored by a shared cache. + * A private cache (e.g. Web Browser) may store the response. + * + * The resulting cache-control headers will be chosen from the 'private' set of directives. + * + * @see https://docs.silverstripe.org/en/developer_guides/performance/http_cache_headers/ + * @param bool $force Force the cache to private even if it's forced public + * @return $this + */ + public function privateCache($force = false) + { + // Only execute this if its forcing level is high enough + if ($this->applyChangeLevel(self::LEVEL_PRIVATE, $force)) { + $this->setState(self::STATE_PRIVATE); + } + return $this; + } + + /** + * Advanced way to set cache control header to a cacheable state. + * Indicates that the response may be cached by any cache. (eg: CDNs, Proxies, Web browsers) + * + * The resulting cache-control headers will be chosen from the 'private' set of directives. + * + * @see https://docs.silverstripe.org/en/developer_guides/performance/http_cache_headers/ + * @param bool $force Force the cache to public even if it's private, unless it's been forced private + * @return $this + */ + public function publicCache($force = false) + { + // Only execute this if its forcing level is high enough + if (!$this->applyChangeLevel(self::LEVEL_PUBLIC, $force)) { + $this->setState(self::STATE_PUBLIC); + } + return $this; + } + + /** + * Generate and add the `Cache-Control` header to a response object + * + * @param HTTPResponse $response + * + * @return $this + */ + public function applyToResponse($response) + { + $headers = $this->generateHeaders(); + foreach ($headers as $name => $value) { + $response->addHeader($name, $value); + } + return $this; + } + + /** + * Generate the cache header + * + * @return string + */ + protected function generateCacheHeader() + { + $cacheControl = []; + foreach ($this->getDirectives() as $directive => $value) { + if ($value === true) { + $cacheControl[] = $directive; + } else { + $cacheControl[] = $directive . '=' . $value; + } + } + return implode(', ', $cacheControl); + } + + /** + * Generate all headers to output + * + * @return array + */ + public function generateHeaders() + { + return [ + 'Cache-Control' => $this->generateCacheHeader(), + ]; + } + + /** + * Reset registered http cache control and force a fresh instance to be built + */ + public static function reset() + { + Injector::inst()->unregisterNamedObject(__CLASS__); + } } From 37343cf0e22fc3bed8c46de3b7330ac79041cac1 Mon Sep 17 00:00:00 2001 From: Daniel Hensby Date: Tue, 12 Jun 2018 12:52:34 +0100 Subject: [PATCH 04/34] Use veradic argument for HTTP::combineVary --- src/Control/HTTP.php | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/Control/HTTP.php b/src/Control/HTTP.php index 1227dda52..3ed7d8fae 100644 --- a/src/Control/HTTP.php +++ b/src/Control/HTTP.php @@ -603,18 +603,18 @@ class HTTP /** * Combine vary strings * - * @param string $vary,... Each vary as a separate arg + * @param string[] $varies Each vary as a separate arg * @return string */ - protected static function combineVary($vary) + protected static function combineVary(...$varies) { - $varies = []; - foreach (func_get_args() as $arg) { - $argVaries = array_filter(preg_split("/\s*,\s*/", trim($arg))); + $cleanVaries = []; + foreach ($varies as $vary) { + $argVaries = array_filter(preg_split("/\s*,\s*/", trim($vary))); if ($argVaries) { - $varies = array_merge($varies, $argVaries); + $cleanVaries = array_merge($cleanVaries, $argVaries); } } - return implode(', ', array_unique($varies)); + return implode(', ', array_unique($cleanVaries)); } } From 6bb69d1ae5f9c1458957527e5812d2ef7c0a579b Mon Sep 17 00:00:00 2001 From: Daniel Hensby Date: Tue, 12 Jun 2018 12:53:07 +0100 Subject: [PATCH 05/34] Throw caught exceptions in HTTPCacheControlMiddleware::process --- src/Control/Middleware/HTTPCacheControlMiddleware.php | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/Control/Middleware/HTTPCacheControlMiddleware.php b/src/Control/Middleware/HTTPCacheControlMiddleware.php index 9db482432..c7c5c06d3 100644 --- a/src/Control/Middleware/HTTPCacheControlMiddleware.php +++ b/src/Control/Middleware/HTTPCacheControlMiddleware.php @@ -33,6 +33,7 @@ class HTTPCacheControlMiddleware implements HTTPMiddleware, Resettable * @param HTTPRequest $request * @param callable $delegate * @return HTTPResponse + * @throws HTTPResponse_Exception */ public function process(HTTPRequest $request, callable $delegate) { @@ -50,6 +51,9 @@ class HTTPCacheControlMiddleware implements HTTPMiddleware, Resettable } HTTP::add_cache_headers($response); + if (isset($ex)) { + throw $ex; + } return $response; } From e76cf935143ca657baae34a60826243b079a98c0 Mon Sep 17 00:00:00 2001 From: Daniel Hensby Date: Tue, 12 Jun 2018 14:26:19 +0100 Subject: [PATCH 06/34] Migrate tests --- .../HTTPCacheControlIntegrationTest.php | 110 ++++++++++++++++++ .../RuleController.php | 45 +++++++ .../SessionController.php | 81 +++++++++++++ tests/php/Control/HTTPTest.php | 54 ++++++--- .../HTTPCacheControlMiddlewareTest.php | 69 +++++++++++ 5 files changed, 343 insertions(+), 16 deletions(-) create mode 100644 tests/php/Control/HTTPCacheControlIntegrationTest.php create mode 100644 tests/php/Control/HTTPCacheControlIntegrationTest/RuleController.php create mode 100644 tests/php/Control/HTTPCacheControlIntegrationTest/SessionController.php create mode 100644 tests/php/Control/Middleware/HTTPCacheControlMiddlewareTest.php diff --git a/tests/php/Control/HTTPCacheControlIntegrationTest.php b/tests/php/Control/HTTPCacheControlIntegrationTest.php new file mode 100644 index 000000000..e443aa1ec --- /dev/null +++ b/tests/php/Control/HTTPCacheControlIntegrationTest.php @@ -0,0 +1,110 @@ +remove(HTTP::class, 'disable_http_cache'); + HTTPCacheControlMiddleware::reset(); + } + + public function testFormCSRF() + { + // CSRF sets caching to disabled + $response = $this->get('HTTPCacheControlIntegrationTest_SessionController/showform'); + $header = $response->getHeader('Cache-Control'); + $this->assertFalse($response->isError()); + $this->assertNotContains('public', $header); + $this->assertNotContains('private', $header); + $this->assertContains('no-cache', $header); + $this->assertContains('no-store', $header); + $this->assertContains('must-revalidate', $header); + } + + public function testPublicForm() + { + // Public forms (http get) allow public caching + $response = $this->get('HTTPCacheControlIntegrationTest_SessionController/showpublicform'); + $header = $response->getHeader('Cache-Control'); + $this->assertFalse($response->isError()); + $this->assertContains('public', $header); + $this->assertContains('must-revalidate', $header); + $this->assertNotContains('no-cache', $response->getHeader('Cache-Control')); + $this->assertNotContains('no-store', $response->getHeader('Cache-Control')); + } + + public function testPrivateActionsError() + { + // disallowed private actions don't cache + $response = $this->get('HTTPCacheControlIntegrationTest_SessionController/privateaction'); + $header = $response->getHeader('Cache-Control'); + $this->assertTrue($response->isError()); + $this->assertContains('no-cache', $header); + $this->assertContains('no-store', $header); + $this->assertContains('must-revalidate', $header); + } + + public function testPrivateActionsAuthenticated() + { + $this->logInWithPermission('ADMIN'); + // Authenticated actions are private cache + $response = $this->get('HTTPCacheControlIntegrationTest_SessionController/privateaction'); + $header = $response->getHeader('Cache-Control'); + $this->assertFalse($response->isError()); + $this->assertContains('private', $header); + $this->assertContains('must-revalidate', $header); + $this->assertNotContains('no-cache', $header); + $this->assertNotContains('no-store', $header); + } + + public function testPrivateCache() + { + $response = $this->get('HTTPCacheControlIntegrationTest_RuleController/privateaction'); + $header = $response->getHeader('Cache-Control'); + $this->assertFalse($response->isError()); + $this->assertContains('private', $header); + $this->assertContains('must-revalidate', $header); + $this->assertNotContains('no-cache', $header); + $this->assertNotContains('no-store', $header); + } + + public function testPublicCache() + { + $response = $this->get('HTTPCacheControlIntegrationTest_RuleController/publicaction'); + $header = $response->getHeader('Cache-Control'); + $this->assertFalse($response->isError()); + $this->assertContains('public', $header); + $this->assertContains('must-revalidate', $header); + $this->assertNotContains('no-cache', $header); + $this->assertNotContains('no-store', $header); + $this->assertContains('max-age=9000', $header); + } + + public function testDisabledCache() + { + $response = $this->get('HTTPCacheControlIntegrationTest_RuleController/disabledaction'); + $header = $response->getHeader('Cache-Control'); + $this->assertFalse($response->isError()); + $this->assertNotContains('public', $header); + $this->assertNotContains('private', $header); + $this->assertContains('no-cache', $header); + $this->assertContains('no-store', $header); + $this->assertContains('must-revalidate', $header); + } +} diff --git a/tests/php/Control/HTTPCacheControlIntegrationTest/RuleController.php b/tests/php/Control/HTTPCacheControlIntegrationTest/RuleController.php new file mode 100644 index 000000000..f5e72fc08 --- /dev/null +++ b/tests/php/Control/HTTPCacheControlIntegrationTest/RuleController.php @@ -0,0 +1,45 @@ +publicCache(); + } + + public function privateaction() + { + HTTPCacheControlMiddleware::singleton()->privateCache(); + return 'private content'; + } + + public function publicaction() + { + HTTPCacheControlMiddleware::singleton() + ->publicCache() + ->setMaxAge(9000); + return 'public content'; + } + + public function disabledaction() + { + HTTPCacheControlMiddleware::singleton()->disableCache(); + return 'uncached content'; + } +} diff --git a/tests/php/Control/HTTPCacheControlIntegrationTest/SessionController.php b/tests/php/Control/HTTPCacheControlIntegrationTest/SessionController.php new file mode 100644 index 000000000..cc55aafbd --- /dev/null +++ b/tests/php/Control/HTTPCacheControlIntegrationTest/SessionController.php @@ -0,0 +1,81 @@ +publicCache(); + } + + public function getContent() + { + return '

Hello world

'; + } + + public function showform() + { + // Form should be set to private due to CSRF + SecurityToken::enable(); + return $this->renderWith('BlankPage'); + } + + public function showpublicform() + { + // Public form doesn't use CSRF and thus no session usage + SecurityToken::disable(); + return $this->renderWith('BlankPage'); + } + + /** + * @return string + * @throws \SilverStripe\Control\HTTPResponse_Exception + */ + public function privateaction() + { + if (!Permission::check('ANYCODE')) { + $this->httpError(403, 'Not allowed'); + } + return 'ok'; + } + + public function publicaction() + { + return 'Hello!'; + } + + public function Form() + { + $form = new Form( + $this, + 'Form', + new FieldList(new TextField('Name')), + new FieldList(new FormAction('submit', 'Submit')) + ); + $form->setFormMethod('GET'); + return $form; + } +} diff --git a/tests/php/Control/HTTPTest.php b/tests/php/Control/HTTPTest.php index 2945c088d..28609c956 100644 --- a/tests/php/Control/HTTPTest.php +++ b/tests/php/Control/HTTPTest.php @@ -7,6 +7,8 @@ use SilverStripe\Control\Director; use SilverStripe\Control\HTTP; use SilverStripe\Control\HTTPRequest; use SilverStripe\Control\HTTPResponse; +use SilverStripe\Control\Middleware\HTTPCacheControlMiddleware; +use SilverStripe\Core\Config\Config; use SilverStripe\Core\Injector\Injector; use SilverStripe\Core\Kernel; use SilverStripe\Dev\FunctionalTest; @@ -18,31 +20,45 @@ use SilverStripe\Dev\FunctionalTest; */ class HTTPTest extends FunctionalTest { + protected function setUp() + { + parent::setUp(); + // Remove dev-only config + Config::modify()->remove(HTTP::class, 'disable_http_cache'); + HTTPCacheControlMiddleware::reset(); + } public function testAddCacheHeaders() { $body = "

Mysite

"; $response = new HTTPResponse($body, 200); - $this->assertEmpty($response->getHeader('Cache-Control')); - + HTTPCacheControlMiddleware::singleton()->publicCache(); HTTP::set_cache_age(30); HTTP::add_cache_headers($response); $this->assertNotEmpty($response->getHeader('Cache-Control')); - // Ensure max-age is zero for development. /** @var Kernel $kernel */ $kernel = Injector::inst()->get(Kernel::class); - $kernel->setEnvironment(Kernel::DEV); + // Ensure cache headers are set correctly when disabled via config (e.g. when dev) + Config::modify()->set(HTTP::class, 'disable_http_cache', true); + HTTPCacheControlMiddleware::reset(); + HTTPCacheControlMiddleware::singleton()->publicCache(); + HTTP::set_cache_age(30); $response = new HTTPResponse($body, 200); HTTP::add_cache_headers($response); - $this->assertContains('max-age=0', $response->getHeader('Cache-Control')); + $this->assertContains('no-cache', $response->getHeader('Cache-Control')); + $this->assertContains('no-store', $response->getHeader('Cache-Control')); + $this->assertContains('must-revalidate', $response->getHeader('Cache-Control')); // Ensure max-age setting is respected in production. - $kernel->setEnvironment(Kernel::LIVE); + Config::modify()->remove(HTTP::class, 'disable_http_cache'); + HTTPCacheControlMiddleware::reset(); + HTTPCacheControlMiddleware::singleton()->publicCache(); + HTTP::set_cache_age(30); $response = new HTTPResponse($body, 200); HTTP::add_cache_headers($response); - $this->assertContains('max-age=30', explode(', ', $response->getHeader('Cache-Control'))); + $this->assertContains('max-age=30', $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). @@ -51,35 +67,41 @@ class HTTPTest extends FunctionalTest 'Pragma' => 'no-cache', 'Cache-Control' => 'max-age=0, no-cache, no-store', ); + HTTPCacheControlMiddleware::reset(); + HTTPCacheControlMiddleware::singleton()->publicCache(); + HTTP::set_cache_age(30); $response = new HTTPResponse($body, 200); foreach ($headers as $name => $value) { $response->addHeader($name, $value); } + // Expect a warning if the header is already set + $this->expectException( + \PHPUnit_Framework_Error_Warning::class, + 'Cache-Control header has already been set. ' + . 'Please use HTTPCacheControl API to set caching options instead.' + ); + HTTP::add_cache_headers($response); - foreach ($headers as $name => $value) { - $this->assertEquals($value, $response->getHeader($name)); - } } public function testConfigVary() { - /** @var Kernel $kernel */ - $kernel = Injector::inst()->get(Kernel::class); $body = "

Mysite

"; $response = new HTTPResponse($body, 200); - $kernel->setEnvironment(Kernel::LIVE); HTTP::set_cache_age(30); HTTP::add_cache_headers($response); $v = $response->getHeader('Vary'); $this->assertNotEmpty($v); - $this->assertContains("Cookie", $v); $this->assertContains("X-Forwarded-Protocol", $v); - $this->assertContains("User-Agent", $v); - $this->assertContains("Accept", $v); + $this->assertContains("X-Requested-With", $v); + $this->assertNotContains("Cookie", $v); + $this->assertNotContains("User-Agent", $v); + $this->assertNotContains("Accept", $v); HTTP::config()->update('vary', ''); + HTTPCacheControlMiddleware::reset(); $response = new HTTPResponse($body, 200); HTTP::add_cache_headers($response); diff --git a/tests/php/Control/Middleware/HTTPCacheControlMiddlewareTest.php b/tests/php/Control/Middleware/HTTPCacheControlMiddlewareTest.php new file mode 100644 index 000000000..af4e1a23f --- /dev/null +++ b/tests/php/Control/Middleware/HTTPCacheControlMiddlewareTest.php @@ -0,0 +1,69 @@ +assertTrue($this->isDisabled($hcc), 'caching starts as disabled'); + + $hcc->enableCache(); + $this->assertFalse($this->isDisabled($hcc)); + + $hcc->publicCache(); + $this->assertTrue($this->isPublic($hcc), 'public can be set at start'); + + $hcc->privateCache(); + $this->assertTrue($this->isPrivate($hcc), 'private overrides public'); + + $hcc->publicCache(); + $this->assertFalse($this->isPublic($hcc), 'public does not overrides private'); + + $hcc->disableCache(); + $this->assertTrue($this->isDisabled($hcc), 'disabled overrides private'); + + $hcc->privateCache(); + $this->assertFalse($this->isPrivate($hcc), 'private does not override disabled'); + + $hcc->enableCache(true); + $this->assertFalse($this->isDisabled($hcc)); + + $hcc->publicCache(true); + $this->assertTrue($this->isPublic($hcc), 'force-public overrides disabled'); + + $hcc->privateCache(); + $this->assertFalse($this->isPrivate($hcc), 'private does not overrdie force-public'); + + $hcc->privateCache(true); + $this->assertTrue($this->isPrivate($hcc), 'force-private overrides force-public'); + + $hcc->publicCache(true); + $this->assertFalse($this->isPublic($hcc), 'force-public does not override force-private'); + + $hcc->disableCache(true); + $this->assertTrue($this->isDisabled($hcc), 'force-disabled overrides force-private'); + + $hcc->publicCache(true); + $this->assertFalse($this->isPublic($hcc), 'force-public does not overrides force-disabled'); + } + + protected function isPrivate(HTTPCacheControlMiddleware $hcc) + { + return $hcc->hasDirective('private') && !$hcc->hasDirective('public') && !$hcc->hasDirective('no-cache'); + } + + protected function isPublic(HTTPCacheControlMiddleware $hcc) + { + return $hcc->hasDirective('public') && !$hcc->hasDirective('private') && !$hcc->hasDirective('no-cache'); + } + + protected function isDisabled(HTTPCacheControlMiddleware $hcc) + { + return $hcc->hasDirective('no-cache') && !$hcc->hasDirective('private') && !$hcc->hasDirective('public'); + } +} From 7c875918c7bc48d615b24bc959f659afbad7883b Mon Sep 17 00:00:00 2001 From: Daniel Hensby Date: Tue, 12 Jun 2018 14:26:46 +0100 Subject: [PATCH 07/34] FIX make sure we create ETags from the body, not the request --- src/Control/HTTP.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Control/HTTP.php b/src/Control/HTTP.php index 3ed7d8fae..ee981bc3b 100644 --- a/src/Control/HTTP.php +++ b/src/Control/HTTP.php @@ -571,7 +571,7 @@ class HTTP ? $response->getBody() : $response; if ($body) { - return sprintf('"%s"', md5($response)); + return sprintf('"%s"', md5($body)); } return false; } From befd81d0c238125b8b82dc8af69b58a1e639a6c4 Mon Sep 17 00:00:00 2001 From: Daniel Hensby Date: Tue, 12 Jun 2018 17:59:56 +0100 Subject: [PATCH 08/34] FIX Bug with forms being cached --- src/Forms/Form.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Forms/Form.php b/src/Forms/Form.php index 84d6574a4..b2d921a34 100644 --- a/src/Forms/Form.php +++ b/src/Forms/Form.php @@ -7,6 +7,7 @@ use SilverStripe\Control\Controller; use SilverStripe\Control\HasRequestHandler; use SilverStripe\Control\HTTP; use SilverStripe\Control\HTTPRequest; +use SilverStripe\Control\Middleware\HTTPCacheControlMiddleware; use SilverStripe\Control\NullHTTPRequest; use SilverStripe\Control\RequestHandler; use SilverStripe\Control\Session; @@ -884,7 +885,7 @@ class Form extends ViewableData implements HasRequestHandler // If we need to disable cache, do it if ($needsCacheDisabled) { - HTTP::set_cache_age(0); + HTTPCacheControlMiddleware::singleton()->disableCache(); } $attrs = $this->getAttributes(); From aa1ba0ef90109fe084a8d736ed4ca08b32a4f40b Mon Sep 17 00:00:00 2001 From: Damian Mooyman Date: Wed, 13 Jun 2018 13:56:47 +1200 Subject: [PATCH 09/34] Fix inverted condition Remove unnecessary yml block Deprecate HTTP::set_cache_age() --- _config/requestprocessors.yml | 9 --------- src/Control/HTTP.php | 5 ++++- .../Middleware/HTTPCacheControlMiddleware.php | 2 +- tests/php/Control/HTTPTest.php | 19 ++++++++----------- 4 files changed, 13 insertions(+), 22 deletions(-) diff --git a/_config/requestprocessors.yml b/_config/requestprocessors.yml index 056f93e7e..c76471ccd 100644 --- a/_config/requestprocessors.yml +++ b/_config/requestprocessors.yml @@ -47,12 +47,3 @@ SilverStripe\Core\Injector\Injector: properties: ForceSSL: false ForceWWW: false ---- -Name: httpcache-dev -Only: - environment: dev ---- -SilverStripe\Core\Injector\Injector: - SilverStripe\Control\Middleware\HTTPCacheControlMiddleware: - Properties: - Enabled: false diff --git a/src/Control/HTTP.php b/src/Control/HTTP.php index ee981bc3b..e8023f1e7 100644 --- a/src/Control/HTTP.php +++ b/src/Control/HTTP.php @@ -9,6 +9,7 @@ use SilverStripe\Core\Config\Configurable; use SilverStripe\Core\Convert; use InvalidArgumentException; use finfo; +use SilverStripe\Dev\Deprecation; /** * A class with HTTP-related helpers. Like Debug, this is more a bundle of methods than a class. @@ -356,12 +357,14 @@ class HTTP /** * Set the maximum age of this page in web caches, in seconds. * + * @deprecated 4.2..5.0 Use HTTPCacheControlMiddleware::singleton()->setMaxAge($age) instead * @param int $age */ public static function set_cache_age($age) { + Deprecation::notice('5.0', 'Use HTTPCacheControlMiddleware::singleton()->setMaxAge($age) instead'); self::$cache_age = $age; - HTTPCacheControlMiddleware::singleton()->setMaxAge(self::$cache_age); + HTTPCacheControlMiddleware::singleton()->setMaxAge($age); } /** diff --git a/src/Control/Middleware/HTTPCacheControlMiddleware.php b/src/Control/Middleware/HTTPCacheControlMiddleware.php index c7c5c06d3..54adc19f6 100644 --- a/src/Control/Middleware/HTTPCacheControlMiddleware.php +++ b/src/Control/Middleware/HTTPCacheControlMiddleware.php @@ -499,7 +499,7 @@ class HTTPCacheControlMiddleware implements HTTPMiddleware, Resettable public function publicCache($force = false) { // Only execute this if its forcing level is high enough - if (!$this->applyChangeLevel(self::LEVEL_PUBLIC, $force)) { + if ($this->applyChangeLevel(self::LEVEL_PUBLIC, $force)) { $this->setState(self::STATE_PUBLIC); } return $this; diff --git a/tests/php/Control/HTTPTest.php b/tests/php/Control/HTTPTest.php index 28609c956..a99bf6311 100644 --- a/tests/php/Control/HTTPTest.php +++ b/tests/php/Control/HTTPTest.php @@ -2,6 +2,7 @@ namespace SilverStripe\Control\Tests; +use PHPUnit_Framework_Error_Warning; use SilverStripe\Control\Controller; use SilverStripe\Control\Director; use SilverStripe\Control\HTTP; @@ -9,8 +10,6 @@ use SilverStripe\Control\HTTPRequest; use SilverStripe\Control\HTTPResponse; use SilverStripe\Control\Middleware\HTTPCacheControlMiddleware; use SilverStripe\Core\Config\Config; -use SilverStripe\Core\Injector\Injector; -use SilverStripe\Core\Kernel; use SilverStripe\Dev\FunctionalTest; /** @@ -33,18 +32,16 @@ class HTTPTest extends FunctionalTest $body = "

Mysite

"; $response = new HTTPResponse($body, 200); HTTPCacheControlMiddleware::singleton()->publicCache(); - HTTP::set_cache_age(30); + HTTPCacheControlMiddleware::singleton()->setMaxAge(30); HTTP::add_cache_headers($response); $this->assertNotEmpty($response->getHeader('Cache-Control')); - /** @var Kernel $kernel */ - $kernel = Injector::inst()->get(Kernel::class); // Ensure cache headers are set correctly when disabled via config (e.g. when dev) Config::modify()->set(HTTP::class, 'disable_http_cache', true); HTTPCacheControlMiddleware::reset(); HTTPCacheControlMiddleware::singleton()->publicCache(); - HTTP::set_cache_age(30); + HTTPCacheControlMiddleware::singleton()->setMaxAge(30); $response = new HTTPResponse($body, 200); HTTP::add_cache_headers($response); $this->assertContains('no-cache', $response->getHeader('Cache-Control')); @@ -55,7 +52,7 @@ class HTTPTest extends FunctionalTest Config::modify()->remove(HTTP::class, 'disable_http_cache'); HTTPCacheControlMiddleware::reset(); HTTPCacheControlMiddleware::singleton()->publicCache(); - HTTP::set_cache_age(30); + HTTPCacheControlMiddleware::singleton()->setMaxAge(30); $response = new HTTPResponse($body, 200); HTTP::add_cache_headers($response); $this->assertContains('max-age=30', $response->getHeader('Cache-Control')); @@ -69,14 +66,14 @@ class HTTPTest extends FunctionalTest ); HTTPCacheControlMiddleware::reset(); HTTPCacheControlMiddleware::singleton()->publicCache(); - HTTP::set_cache_age(30); + HTTPCacheControlMiddleware::singleton()->setMaxAge(30); $response = new HTTPResponse($body, 200); foreach ($headers as $name => $value) { $response->addHeader($name, $value); } // Expect a warning if the header is already set - $this->expectException( - \PHPUnit_Framework_Error_Warning::class, + $this->expectException(PHPUnit_Framework_Error_Warning::class); + $this->expectExceptionMessage( 'Cache-Control header has already been set. ' . 'Please use HTTPCacheControl API to set caching options instead.' ); @@ -88,7 +85,7 @@ class HTTPTest extends FunctionalTest { $body = "

Mysite

"; $response = new HTTPResponse($body, 200); - HTTP::set_cache_age(30); + HTTPCacheControlMiddleware::singleton()->setMaxAge(30); HTTP::add_cache_headers($response); $v = $response->getHeader('Vary'); From 6f32762268941ba42ad2a129c67172e51cb1a4d3 Mon Sep 17 00:00:00 2001 From: Damian Mooyman Date: Wed, 13 Jun 2018 14:09:31 +1200 Subject: [PATCH 10/34] Fix unit tests --- src/Control/Middleware/HTTPCacheControlMiddleware.php | 2 +- tests/php/Control/HTTPCacheControlIntegrationTest.php | 1 - tests/php/Control/HTTPTest.php | 2 +- 3 files changed, 2 insertions(+), 3 deletions(-) diff --git a/src/Control/Middleware/HTTPCacheControlMiddleware.php b/src/Control/Middleware/HTTPCacheControlMiddleware.php index 54adc19f6..8c5cd88a8 100644 --- a/src/Control/Middleware/HTTPCacheControlMiddleware.php +++ b/src/Control/Middleware/HTTPCacheControlMiddleware.php @@ -92,7 +92,7 @@ class HTTPCacheControlMiddleware implements HTTPMiddleware, Resettable * * @var string */ - protected $state = self::STATE_ENABLED; + protected $state = self::STATE_DISABLED; /** * Forcing level of previous setting; higher number wins diff --git a/tests/php/Control/HTTPCacheControlIntegrationTest.php b/tests/php/Control/HTTPCacheControlIntegrationTest.php index e443aa1ec..29af2024e 100644 --- a/tests/php/Control/HTTPCacheControlIntegrationTest.php +++ b/tests/php/Control/HTTPCacheControlIntegrationTest.php @@ -4,7 +4,6 @@ namespace SilverStripe\Control\Tests; use SilverStripe\Control\HTTP; use SilverStripe\Control\Middleware\HTTPCacheControlMiddleware; -use SilverStripe\Control\Session; use SilverStripe\Control\Tests\HTTPCacheControlIntegrationTest\RuleController; use SilverStripe\Control\Tests\HTTPCacheControlIntegrationTest\SessionController; use SilverStripe\Core\Config\Config; diff --git a/tests/php/Control/HTTPTest.php b/tests/php/Control/HTTPTest.php index a99bf6311..7de7966ee 100644 --- a/tests/php/Control/HTTPTest.php +++ b/tests/php/Control/HTTPTest.php @@ -75,7 +75,7 @@ class HTTPTest extends FunctionalTest $this->expectException(PHPUnit_Framework_Error_Warning::class); $this->expectExceptionMessage( 'Cache-Control header has already been set. ' - . 'Please use HTTPCacheControl API to set caching options instead.' + . 'Please use HTTPCacheControlMiddleware API to set caching options instead.' ); HTTP::add_cache_headers($response); From 3ea98cdb13530c703c85e61f76438977fc06e396 Mon Sep 17 00:00:00 2001 From: Damian Mooyman Date: Wed, 13 Jun 2018 14:50:02 +1200 Subject: [PATCH 11/34] Migrate documentation from 3.x --- .../03_Forms/04_Form_Security.md | 11 + .../08_Performance/02_HTTP_Cache_Headers.md | 227 ++++++++++++++++-- .../09_Security/04_Secure_Coding.md | 9 + docs/en/04_Changelogs/4.2.0.md | 144 +++++++++++ 4 files changed, 369 insertions(+), 22 deletions(-) diff --git a/docs/en/02_Developer_Guides/03_Forms/04_Form_Security.md b/docs/en/02_Developer_Guides/03_Forms/04_Form_Security.md index 93c4c6cf9..a34971a60 100644 --- a/docs/en/02_Developer_Guides/03_Forms/04_Form_Security.md +++ b/docs/en/02_Developer_Guides/03_Forms/04_Form_Security.md @@ -76,6 +76,17 @@ functionality is available as an additional [Spam Protection](https://github.com module if required. The module provides an consistent API for allowing third-party spam protection handlers such as [Recaptcha](http://www.google.com/recaptcha/intro/) and [Mollom](https://mollom.com/) to work within the `Form` API. +## Data disclosure through HTTP Caching (since 4.2.0) + +Forms, and particularly their responses, can contain sensitive data (such as when data is pre-populated or re-posted due +to validation errors). This data can inadvertently be stored either in a user's browser cache or in an intermediary +cache such as a CDN or other caching-proxy. If incorrect `Cache-Conrol` headers are used, private data may be cached and +accessible publicly through the CDN. + +To ensure this doesn't happen SilverStripe adds `Cache-Control: no-store, no-cache, must-revalidate` headers to any +forms that have validators or security tokens (all of them by default) applied to them; this ensures that CDNs +(and browsers) will not cache these pages. + ## Related Documentation * [Security](../security) diff --git a/docs/en/02_Developer_Guides/08_Performance/02_HTTP_Cache_Headers.md b/docs/en/02_Developer_Guides/08_Performance/02_HTTP_Cache_Headers.md index bb3df73c5..946ec67a7 100644 --- a/docs/en/02_Developer_Guides/08_Performance/02_HTTP_Cache_Headers.md +++ b/docs/en/02_Developer_Guides/08_Performance/02_HTTP_Cache_Headers.md @@ -1,7 +1,188 @@ title: HTTP Cache Headers summary: Set the correct HTTP cache headers for your responses. -# Caching Headers +# HTTP Cache Headers + +## Overview + +By default, SilverStripe sends headers which signal to HTTP caches +that the response should be not considered cacheable. +HTTP caches can either be intermediary caches (e.g. CDNs and proxies), or clients (e.g. browsers). +The cache headers sent are `Cache-Control: no-store, no-cache, must-revalidate`; + +HTTP caching can be a great way to speed up your website, but needs to be properly applied. +Getting it wrong can accidentally expose draft pages or other protected content. +The [Google Web Fundamentals](https://developers.google.com/web/fundamentals/performance/optimizing-content-efficiency/http-caching#public_vs_private) +are a great way to learn about HTTP caching. + +## Cache Control Headers + +### Overview + +In order to support developers in making safe choices around HTTP caching, +we're using a `HTTPCacheControl` class to control if a response +should be considered public or private. This is an abstraction on existing +lowlevel APIs like `HTTP::add_cache_headers()` and `SS_HTTPResponse->addHeader()`. + +The `HTTPCacheControl` API makes it easier to express your caching preferences +without running the risk of overriding essential core safety measures. +Most commonly, these APIs will prevent HTTP caching of draft content. + +It will also prevent caching of content generated with an active session, +since the system can't tell whether session data was used to vary the output. +In this case, it's up to the developer to opt-in to caching, +after ensuring that certain execution paths are safe despite of using sessions. + +The system behaviour does not guard against accidentally caching "private" content, +since there are too many variations under which output could be considered private +(e.g. a custom "approval" flag on a comment object). It is up to +the developer to ensure caching is used appropriately there. + +The [api:SilverStripe\Control\Middleware\HTTPCacheControlMiddleware] class supplements the `HTTP` helper class. +It comes with methods which let developers safely interact with the `Cache-Control` header. + +### disableCache() + +Simple way to set cache control header to a non-cacheable state. +Use this method over `privateCache()` if you are unsure about caching details. +Takes precendence over unforced `enableCache()`, `privateCache()` or `publicCache()` calls. + +Removes all state and replaces it with `no-cache, no-store, must-revalidate`. Although `no-store` is sufficient +the others are added under [recommendation from Mozilla](https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Cache-Control#Examples) + +Does not set `private` directive, use `privateCache()` if this is explicitly required +([details](https://developers.google.com/web/fundamentals/performance/optimizing-content-efficiency/http-caching#public_vs_private)) + +### enableCache() + +Simple way to set cache control header to a cacheable state. +Use this method over `publicCache()` if you are unsure about caching details. + +Removes `no-store` and `no-cache` directives; other directives will remain in place. +Use alongside `setMaxAge()` to indicate caching. + +Does not set `public` directive. Usually, `setMaxAge()` is sufficient. Use `publicCache()` if this is explicitly required +([details](https://developers.google.com/web/fundamentals/performance/optimizing-content-efficiency/http-caching#public_vs_private)) + +### privateCache() + +Advanced way to set cache control header to a non-cacheable state. +Indicates that the response is intended for a single user and must not be stored by a shared cache. +A private cache (e.g. Web Browser) may store the response. Also removes `public` as this is a contradictory directive. + +### publicCache() + +Advanced way to set cache control header to a cacheable state. +Indicates that the response may be cached by any cache. (eg: CDNs, Proxies, Web browsers) +Also removes `private` as this is a contradictory directive + +### Priority + +Each of these highlevel methods has a boolean `$force` parameter which determines +their application priority regardless of execution order. +The priority order is as followed, sorted in descending order +(earlier items will overrule later items): + + * `disableCache($force=true)` + * `privateCache($force=true)` + * `publicCache($force=true)` + * `enableCache($force=true)` + * `disableCache()` + * `privateCache()` + * `publicCache()` + * `enableCache()` + +## Cache Control Examples + +### Global opt-in for page content + +Enable caching for all page content (through `Page_Controller`). + +```php +enableCache() + ->setMaxAge(60); // 1 minute + + parent::init(); + } +} +``` + +Note: SilverStripe will still override this preference when a session is active, +a [CSRF token](/developer_guides/forms/form_security) token is present, +or draft content has been requested. + +### Opt-out for a particular controller action + +If a controller output relies on session data, cookies, +permission checks or other triggers for conditional output, +you can disable caching either on a controller level +(through `init()`) or for a particular action. + +```php +disableCache(); + + return $this->myPrivateResponse();; + } +} +``` + +Note: SilverStripe will still override this preference when a session is active, +a [CSRF token](/developer_guides/forms/form_security) token is present, +or draft content has been requested. + +### Global opt-in, ignoring session (advanced) + +This can be helpful in situations where forms are embedded on the website. +SilverStripe will still override this preference when draft content has been requested. +CAUTION: This mode relies on a developer examining each execution path to ensure +that no session data is used to vary output. + +Use case: By default, forms include a [CSRF token](/developer_guides/forms/form_security) +which starts a session with a value that's unique to the visitor, which makes the output uncacheable. +But any subsequent requests by this visitor will also carry a session, leading to uncacheable output +for this visitor. This is the case even if the output does not contain any forms, +and does not vary for this particular visitor. + +```php +enableCache($force=true) // DANGER ZONE + ->setMaxAge(60); // 1 minute + + parent::init(); + } +} +``` + +## Defaults By default, PHP adds caching headers that make the page appear purely dynamic. This isn't usually appropriate for most sites, even ones that are updated reasonably frequently. SilverStripe overrides the default settings with the following @@ -14,39 +195,41 @@ headers: * Since a visitor cookie is set, the site won't be cached by proxies. * Ajax requests are never cached. -## Customizing Cache Headers +## Max Age -### HTTP::set_cache_age +The cache age determines the lifetime of your cache, in seconds. +It only takes effect if you instruct the cache control +that your response is public in the first place (via `enableCache()` or via modifying the `HTTP.cache_control` defaults). + +```php +use SilverStripe\Control\Middleware\HTTPCacheControlMiddleware; +HTTPCacheControlMiddleware::singleton() + ->setMaxAge(60) +``` + +Note that `setMaxAge(0)` is NOT sufficient to disable caching in all cases. + +### Last Modified + +Used to set the modification date to something more recent than the default. [api:DataObject::__construct] calls +[api:HTTP::register_modification_date(] whenever a record comes from the database ensuring the newest date is present. ```php use SilverStripe\Control\HTTP; - -HTTP::set_cache_age(0); -``` - -Used to set the max-age component of the cache-control line, in seconds. Set it to 0 to disable caching; the "no-cache" -clause in `Cache-Control` and `Pragma` will be included. - -### HTTP::register_modification_date - - -```php HTTP::register_modification_date('2014-10-10'); ``` -Used to set the modification date to something more recent than the default. [DataObject::__construct](api:SilverStripe\ORM\DataObject::__construct) calls -[HTTP::register_modification_date(](api:SilverStripe\Control\HTTP::register_modification_date() whenever a record comes from the database ensuring the newest date is present. +### Vary -### Vary: cache header - -By default, SilverStripe will output a `Vary` header (used by upstream caches for determining uniqueness) -that looks like +A `Vary` header tells caches which aspects of the response should be considered +when calculating a cache key, usually in addition to the full URL path. +By default, SilverStripe will output a `Vary` header with the following content: ``` -Cookie, X-Forwarded-Protocol, User-Agent, Accept +Vary: X-Requested-With, X-Forwarded-Protocol ``` -To change the value of the `Vary` header, you can change this value by specifying the header in configuration +To change the value of the `Vary` header, you can change this value by specifying the header in configuration. ```yml SilverStripe\Control\HTTP: diff --git a/docs/en/02_Developer_Guides/09_Security/04_Secure_Coding.md b/docs/en/02_Developer_Guides/09_Security/04_Secure_Coding.md index d2a892464..eebb28ec9 100644 --- a/docs/en/02_Developer_Guides/09_Security/04_Secure_Coding.md +++ b/docs/en/02_Developer_Guides/09_Security/04_Secure_Coding.md @@ -770,6 +770,15 @@ class MySecureController extends Controller } ``` +## HTTP Caching Headers + +Caching is hard. If you get it wrong, private or draft content might leak +to unauthenticated users. We have created an abstraction which allows you to express +your intent around HTTP caching without worrying too much about the details. +See [/developer_guides/performances/http_cache_headers](Developer Guides > Performance > HTTP Cache Headers) +for details on how to apply caching safely, and read Google's +[Web Fundamentals on Caching](https://developers.google.com/web/fundamentals/performance/optimizing-content-efficiency/http-caching). + ## Related * [http://silverstripe.org/security-releases/](http://silverstripe.org/security-releases/) diff --git a/docs/en/04_Changelogs/4.2.0.md b/docs/en/04_Changelogs/4.2.0.md index 9d53b78ad..090975968 100644 --- a/docs/en/04_Changelogs/4.2.0.md +++ b/docs/en/04_Changelogs/4.2.0.md @@ -201,3 +201,147 @@ SilverStripe\Core\Injector\Injector: args: disable-container: true ``` + +### HTTP Cache Header changes + +#### Overview + +In order to support developers in making safe choices around HTTP caching, +we've introduced a `HTTPCacheControlMiddleware` class to control if a response +should be considered public or private. This is an abstraction on existing +lowlevel APIs like `HTTP::add_cache_headers()` and `SS_HTTPResponse->addHeader()`. + +This change introduces smaller but necessary changes to HTTP caching headers +sent by SilverStripe. If you are relying on HTTP caching in your implementation, +or use modules such as [silverstripe/controllerpolicy](https://github.com/silverstripe/silverstripe-controllerpolicy), +please review the implications of these changes below. + +In short, these APIs make it easier to express your caching preferences +without running the risk of overriding essential core safety measures. +Most commonly, these APIs will prevent HTTP caching of draft content. + +It will also prevent caching of content generated with an active session, +since the system can't tell whether session data was used to vary the output. +In this case, it's up to the developer to opt-in to caching, +after ensuring that certain execution paths are safe despite of using sessions. + +The system behaviour does not guard against accidentally caching "private" content, +since there are too many variations under which output could be considered private +(e.g. a custom "approval" flag on a comment object). It is up to +the developer to ensure caching is used appropriately there. + +By default, SilverStripe sends headers which signal to HTTP caches +that the response should be considered not cacheable. + +See [Developer Guide: Performance > HTTP Cache Headers](/developer_guide/performance/http_cache_headers) +for details on the new API. + +#### Example Usage + +##### Global opt-in for page content + +Enable caching for all page content (through `Page_Controller`). + +```diff +enableCache() ++ ->setMaxAge(60); // 1 minute + + parent::init(); + } +} +``` + +Note: SilverStripe will still override this preference when a session is active, +a [CSRF token](/developer_guides/forms/form_security) token is present, +or draft content has been requested. + +##### Opt-out for a particular controller action + +If a controller output relies on session data, cookies, +permission checks or other triggers for conditional output, +you can disable caching either on a controller level +(through `init()`) or for a particular action. + +```diff +disableCache(); + + return $this->myPrivateResponse(); + } +} +``` + +Note: SilverStripe will still override this preference when a session is active, +a [CSRF token](/developer_guides/forms/form_security) token is present, +or draft content has been requested. + +##### Global opt-in, ignoring session (advanced) + +This can be helpful in situations where forms are embedded on the website. +SilverStripe will still override this preference when draft content has been requested. +CAUTION: This mode relies on a developer examining each execution path to ensure +that no session data is used to vary output. + +Use case: By default, forms include a [CSRF token](/developer_guides/forms/form_security) +which starts a session with a value that's unique to the visitor, which makes the output uncacheable. +But any subsequent requests by this visitor will also carry a session, leading to uncacheable output +for this visitor. This is the case even if the output does not contain any forms, +and does not vary for this particular visitor. + +```diff +enableCache($force=true) // DANGER ZONE ++ ->setMaxAge(60); // 1 minute + + parent::init(); + } +} +``` + +#### Detailed Cache-Control Changes: + + * Added `Cache-Control: no-store` header to default responses, + to prevent intermediary HTTP proxies (e.g. CDNs) from caching unless developers opt-in + * Removed `Cache-Control: no-transform` header from default responses + * Removed `Vary: Cookie` as an unreliable cache buster, + rely on the existing `Cache-Control: no-store` defaults instead + * Removed `Vary: Accept`, since it's very uncommon to vary content on + the `Accept` headers submitted through the request, + and it can significantly decrease the likelyhood of a cache hit. + Note this is different from `Vary: Accept-Encoding`, + which is important for compression (e.g. gzip), and usually added by + other layers such as Apache's mod_gzip. From 687d0a6af1b435064fb112a6d3b03a04a4038bc4 Mon Sep 17 00:00:00 2001 From: Damian Mooyman Date: Wed, 13 Jun 2018 17:56:47 +1200 Subject: [PATCH 12/34] Refactor everything out of HTTP and into separate middlewares --- _config/config.yml | 12 +- _config/requestprocessors.yml | 1 + src/Control/HTTP.php | 230 +++++----------- src/Control/HTTPResponse.php | 2 +- src/Control/Middleware/ETagMiddleware.php | 102 ++++++++ .../Middleware/HTTPCacheControlMiddleware.php | 245 ++++++++++++++++-- src/ORM/DataObject.php | 4 +- .../HTTPCacheControlIntegrationTest.php | 4 +- tests/php/Control/HTTPTest.php | 14 +- .../HTTPCacheControlMiddlewareTest.php | 10 + 10 files changed, 429 insertions(+), 195 deletions(-) create mode 100644 src/Control/Middleware/ETagMiddleware.php diff --git a/_config/config.yml b/_config/config.yml index 1d2a634fb..4f1e04983 100644 --- a/_config/config.yml +++ b/_config/config.yml @@ -1,12 +1,6 @@ --- Name: coreconfig --- -SilverStripe\Control\HTTP: - cache_control: - no-cache: true - no-store: true - must-revalidate: true - vary: "X-Requested-With, X-Forwarded-Protocol" SilverStripe\Core\Manifest\VersionProvider: modules: silverstripe/framework: Framework @@ -15,5 +9,7 @@ Name: httpconfig-dev Only: environment: dev --- -SilverStripe\Control\HTTP: - disable_http_cache: true +# Set dev level to disabled with a higher forcing level +SilverStripe\Control\Middleware\HTTPCacheControlMiddleware: + defaultState: 'disabled' + defaultForcingLevel: 3 diff --git a/_config/requestprocessors.yml b/_config/requestprocessors.yml index c76471ccd..46917c8db 100644 --- a/_config/requestprocessors.yml +++ b/_config/requestprocessors.yml @@ -11,6 +11,7 @@ SilverStripe\Core\Injector\Injector: SessionMiddleware: '%$SilverStripe\Control\Middleware\SessionMiddleware' RequestProcessorMiddleware: '%$SilverStripe\Control\RequestProcessor' FlushMiddleware: '%$SilverStripe\Control\Middleware\FlushMiddleware' + ETagMiddleware: '%$SilverStripe\Control\Middleware\ETagMiddleware' HTTPCacheControleMiddleware: '%$SilverStripe\Control\Middleware\HTTPCacheControlMiddleware' CanonicalURLMiddleware: '%$SilverStripe\Control\Middleware\CanonicalURLMiddleware' SilverStripe\Control\Middleware\AllowedHostsMiddleware: diff --git a/src/Control/HTTP.php b/src/Control/HTTP.php index e8023f1e7..0277e9ed6 100644 --- a/src/Control/HTTP.php +++ b/src/Control/HTTP.php @@ -3,12 +3,14 @@ namespace SilverStripe\Control; use SilverStripe\Assets\File; +use SilverStripe\Control\Middleware\ETagMiddleware; use SilverStripe\Control\Middleware\HTTPCacheControlMiddleware; use SilverStripe\Core\Config\Config; use SilverStripe\Core\Config\Configurable; use SilverStripe\Core\Convert; use InvalidArgumentException; use finfo; +use SilverStripe\Core\Injector\Injector; use SilverStripe\Dev\Deprecation; /** @@ -19,30 +21,35 @@ class HTTP use Configurable; /** + * @deprecated 4.2..5.0 Use HTTPCacheControlMiddleware::singleton()->setMaxAge($age) instead * @var int */ protected static $cache_age = 0; /** + * @deprecated 4.2..5.0 Handled by HTTPCacheControlMiddleware * @var int */ protected static $modification_date = null; /** + * @deprecated 4.2..5.0 Handled by ETagMiddleware * @var string */ protected static $etag = null; /** * @config - * * @var bool + * @deprecated 4.2..5.0 'HTTP.cache_ajax_requests config is deprecated. + * Use HTTPCacheControlMiddleware::disableCache() instead' */ - private static $cache_ajax_requests = true; + private static $cache_ajax_requests = false; /** * @config * @var bool + * @deprecated 4.2..5.0 Use HTTPCacheControlMiddleware.defaultState/.defaultForcingLevel instead */ private static $disable_http_cache = false; @@ -66,6 +73,7 @@ class HTTP /** * Vary string; A comma separated list of var header names * + * @deprecated 4.2..5.0 Handled by HTTPCacheMiddleware instead * @config * @var string|null */ @@ -328,7 +336,6 @@ class HTTP * commonly known MIME types. * * @param string $filename - * * @return string */ public static function get_mime_type($filename) @@ -369,32 +376,33 @@ class HTTP /** * @param string $dateString + * @deprecated 4.2..5.0 Use HTTPCacheControlMiddleware::registerModificationDate() instead */ public static function register_modification_date($dateString) { - $timestamp = strtotime($dateString); - if ($timestamp > self::$modification_date) { - self::$modification_date = $timestamp; - } + Deprecation::notice('5.0', 'Use HTTPCacheControlMiddleware::registerModificationDate() instead'); + HTTPCacheControlMiddleware::singleton()->registerModificationDate($dateString); } /** * @param int $timestamp + * @deprecated 4.2..5.0 Use HTTPCacheControlMiddleware::registerModificationDate() instead */ public static function register_modification_timestamp($timestamp) { - if ($timestamp > self::$modification_date) { - self::$modification_date = $timestamp; - } + Deprecation::notice('5.0', 'Use HTTPCacheControlMiddleware::registerModificationDate() instead'); + HTTPCacheControlMiddleware::singleton()->registerModificationDate($timestamp); } /** + * @deprecated 4.2..5.0 Use ETagMiddleware instead * @param string $etag */ public static function register_etag($etag) { - if (0 !== strpos($etag, '"')) { - $etag = sprintf('"%s"', $etag); + Deprecation::notice('5.0', 'Use ETagMiddleware instead'); + if (strpos($etag, '"') !== 0) { + $etag = "\"{$etag}\""; } self::$etag = $etag; } @@ -409,24 +417,21 @@ class HTTP * Omitting the $body argument or passing a string is deprecated; in these cases, the headers are * output directly. * - * @param HTTPResponse $body + * @param HTTPResponse $response + * @deprecated 4.2..5.0 Headers are added automatically by HTTPCacheControlMiddleware instead. */ - public static function add_cache_headers($body = null) + public static function add_cache_headers($response = null) { - // Validate argument - if ($body && !($body instanceof HTTPResponse)) { - user_error("HTTP::add_cache_headers() must be passed an HTTPResponse object", E_USER_WARNING); - $body = null; - } + Deprecation::notice('5.0', 'Headers are added automatically by HTTPCacheControlMiddleware instead.'); - // The headers have been sent and we don't have an HTTPResponse object to attach things to; no point in - // us trying. - if (headers_sent() && !$body) { + // Ensure a valid response object is provided + if (!$response instanceof HTTPResponse) { + user_error("HTTP::add_cache_headers() must be passed an HTTPResponse object", E_USER_WARNING); return; } // Warn if already assigned cache-control headers - if ($body && $body->getHeader('Cache-Control')) { + if ($response->getHeader('Cache-Control')) { trigger_error( 'Cache-Control header has already been set. ' . 'Please use HTTPCacheControlMiddleware API to set caching options instead.', @@ -435,157 +440,70 @@ class HTTP return; } + // Ensure a valid request object exists in the current context + if (!Injector::inst()->has(HTTPRequest::class)) { + user_error("HTTP::add_cache_headers() cannot work without a current HTTPRequest object", E_USER_WARNING); + return; + } + + /** @var HTTPRequest $request */ + $request = Injector::inst()->get(HTTPRequest::class); + $config = Config::forClass(__CLASS__); // Get current cache control state - $cacheControl = HTTPCacheControlMiddleware::singleton(); + $cacheControlMiddleware = HTTPCacheControlMiddleware::singleton(); + $etagMiddleware = ETagMiddleware::singleton(); // if http caching is disabled by config, disable it - used on dev environments due to frequently changing - // templates and other data. will be overridden by forced publicCache() or privateCache() calls + // templates and other data. will be overridden by forced publicCache(true) or privateCache(true) calls if ($config->get('disable_http_cache')) { - $cacheControl->disableCache(); + Deprecation::notice('5.0', 'Use HTTPCacheControlMiddleware.defaultState/.defaultForcingLevel instead'); + $cacheControlMiddleware->disableCache(); } - // Populate $responseHeaders with all the headers that we want to build - $responseHeaders = []; - // if no caching ajax requests, disable ajax if is ajax request if (!$config->get('cache_ajax_requests') && Director::is_ajax()) { - $cacheControl->disableCache(); + Deprecation::notice( + '5.0', + 'HTTP.cache_ajax_requests config is deprecated. Use HTTPCacheControlMiddleware::disableCache() instead' + ); + $cacheControlMiddleware->disableCache(); } - // Errors disable cache (unless some errors are cached intentionally by usercode) - if ($body && $body->isError()) { - // Even if publicCache(true) is specfied, errors will be uncachable - $cacheControl->disableCache(true); - } - - // split the current vary header into it's parts and merge it with the config settings - // to create a list of unique vary values + // Pass vary to middleware $configVary = $config->get('vary'); - $bodyVary = $body ? $body->getHeader('Vary') : ''; - $vary = self::combineVary($configVary, $bodyVary); - if ($vary) { - $responseHeaders['Vary'] = $vary; - } - - // deal with IE6-IE8 problems with https and no-cache - $contentDisposition = null; - if ($body) { - // Grab header for checking. Unfortunately HTTPRequest uses a mistyped variant. - $contentDisposition = $body->getHeader('Content-Disposition'); - } - - if ($body && - Director::is_https() && - isset($_SERVER['HTTP_USER_AGENT']) && - strstr($_SERVER['HTTP_USER_AGENT'], 'MSIE') == true && - strstr($contentDisposition, 'attachment;') == true && - ($cacheControl->hasDirective('no-cache') || $cacheControl->hasDirective('no-store')) - ) { - // IE6-IE8 have problems saving files when https and no-cache/no-store are used - // (http://support.microsoft.com/kb/323308) - // Note: this is also fixable by ticking "Do not save encrypted pages to disk" in advanced options. - $cacheControl->privateCache(true); + if ($configVary) { + Deprecation::notice('5.0', 'Use HTTPCacheControlMiddleware.defaultVary instead'); + $cacheControlMiddleware->addVary($configVary); } + // Set modification date if (self::$modification_date) { - $responseHeaders["Last-Modified"] = self::gmt_date(self::$modification_date); + Deprecation::notice('5.0', 'Use HTTPCacheControlMiddleware::registerModificationDate() instead'); + $cacheControlMiddleware->registerModificationDate(self::$modification_date); } - // if we can store the cache responses we should generate and send etags - if (!$cacheControl->hasDirective('no-store')) { - // Chrome ignores Varies when redirecting back (http://code.google.com/p/chromium/issues/detail?id=79758) - // which means that if you log out, you get redirected back to a page which Chrome then checks against - // last-modified (which passes, getting a 304) - // when it shouldn't be trying to use that page at all because it's the "logged in" version. - // By also using and etag that includes both the modification date and all the varies - // values which we also check against we can catch this and not return a 304 - $etag = self::generateETag($body); - if ($etag) { - $responseHeaders['ETag'] = $etag; - - // 304 response detection - if (isset($_SERVER['HTTP_IF_NONE_MATCH'])) { - // As above, only 304 if the last request had all the same varies values - // (or the etag isn't passed as part of the request - but with chrome it always is) - $matchesEtag = $_SERVER['HTTP_IF_NONE_MATCH'] == $etag; - - if ($matchesEtag) { - if ($body) { - $body->setStatusCode(304); - $body->setBody(''); - } else { - // this is wrong, we need to send the same vary headers and so on - header('HTTP/1.0 304 Not Modified'); - die(); - } - } - } - } + // Ensure deprecated $etag property is assigned + if (self::$etag && !$cacheControlMiddleware->hasDirective('no-store') && !$response->getHeader('ETag')) { + Deprecation::notice('5.0', 'Etag should not be set explicitly'); + $response->addHeader('ETag', self::$etag); } - if ($cacheControl->hasDirective('max-age')) { - $expires = time() + $cacheControl->getDirective('max-age'); - $responseHeaders["Expires"] = self::gmt_date($expires); - } - - // etag needs to be a quoted string according to HTTP spec - if (!empty($responseHeaders['ETag']) && 0 !== strpos($responseHeaders['ETag'], '"')) { - $responseHeaders['ETag'] = sprintf('"%s"', $responseHeaders['ETag']); - } - - // Merge with cache control headers - $responseHeaders = array_merge($responseHeaders, $cacheControl->generateHeaders()); - - // 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) { - // 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"); - } - } + // Run middleware + $etagMiddleware->process($request, function (HTTPRequest $request) use ($cacheControlMiddleware, $response) { + return $cacheControlMiddleware->process($request, function (HTTPRequest $request) use ($response) { + return $response; + }); + }); } - - /** - * @param HTTPResponse|string $response - * - * @return string|false - */ - protected static function generateETag($response) - { - // Explicit etag - if (self::$etag) { - return self::$etag; - } - - // Existing e-tag - if ($response instanceof HTTPResponse && $response->getHeader('ETag')) { - return $response->getHeader('ETag'); - } - - // Generate etag from body - $body = $response instanceof HTTPResponse - ? $response->getBody() - : $response; - if ($body) { - return sprintf('"%s"', md5($body)); - } - return false; - } - - /** * Return an {@link http://www.faqs.org/rfcs/rfc2822 RFC 2822} date in the GMT timezone (a timestamp * is always in GMT: the number of seconds since January 1 1970 00:00:00 GMT) * * @param int $timestamp - * + * @deprecated 4.2..5.0 Inline if you need this * @return string */ public static function gmt_date($timestamp) @@ -602,22 +520,4 @@ class HTTP { return self::$cache_age; } - - /** - * Combine vary strings - * - * @param string[] $varies Each vary as a separate arg - * @return string - */ - protected static function combineVary(...$varies) - { - $cleanVaries = []; - foreach ($varies as $vary) { - $argVaries = array_filter(preg_split("/\s*,\s*/", trim($vary))); - if ($argVaries) { - $cleanVaries = array_merge($cleanVaries, $argVaries); - } - } - return implode(', ', array_unique($cleanVaries)); - } } diff --git a/src/Control/HTTPResponse.php b/src/Control/HTTPResponse.php index f26a5f50a..987162e65 100644 --- a/src/Control/HTTPResponse.php +++ b/src/Control/HTTPResponse.php @@ -217,7 +217,7 @@ class HTTPResponse * Return the HTTP header of the given name. * * @param string $header - * @returns string + * @return string */ public function getHeader($header) { diff --git a/src/Control/Middleware/ETagMiddleware.php b/src/Control/Middleware/ETagMiddleware.php new file mode 100644 index 000000000..cfc360340 --- /dev/null +++ b/src/Control/Middleware/ETagMiddleware.php @@ -0,0 +1,102 @@ +getHeader('Cache-Control'); + if ($cacheControl && strstr($cacheControl, 'no-store')) { + return $response; + } + + // Generate, assign, and conditionally check etag + $etag = $this->generateETag($response); + if ($etag) { + $response->addHeader('ETag', $etag); + + // Check if we have a match + $ifNoneMatch = $request->getHeader('If-None-Match'); + if ($ifNoneMatch && $ifNoneMatch === $etag) { + return $this->sendNotModified($request, $response); + } + } + + // Check If-Modified-Since + $ifModifiedSince = $request->getHeader('If-Modified-Since'); + $lastModified = $response->getHeader('Last-Modified'); + if ($ifModifiedSince && $lastModified && strtotime($ifModifiedSince) >= strtotime($lastModified)) { + return $this->sendNotModified($request, $response); + } + + return $response; + } + + /** + * @param HTTPResponse|string $response + * @return string|false + */ + protected function generateETag(HTTPResponse $response) + { + // Existing e-tag + if ($response instanceof HTTPResponse && $response->getHeader('ETag')) { + return $response->getHeader('ETag'); + } + + // Generate etag from body + $body = $response instanceof HTTPResponse + ? $response->getBody() + : $response; + if ($body) { + return sprintf('"%s"', md5($body)); + } + return false; + } + + /** + * Sent not-modified response + * + * @param HTTPRequest $request + * @param HTTPResponse $response + * @return mixed + */ + protected function sendNotModified(HTTPRequest $request, HTTPResponse $response) + { + // 304 is invalid for destructive requests + if (in_array($request->httpMethod(), ['POST', 'DELETE', 'PUT'])) { + $response->setStatusCode(412); + } else { + $response->setStatusCode(304); + } + $response->setBody(''); + return $response; + } +} diff --git a/src/Control/Middleware/HTTPCacheControlMiddleware.php b/src/Control/Middleware/HTTPCacheControlMiddleware.php index 8c5cd88a8..78aa2540a 100644 --- a/src/Control/Middleware/HTTPCacheControlMiddleware.php +++ b/src/Control/Middleware/HTTPCacheControlMiddleware.php @@ -11,6 +11,7 @@ use SilverStripe\Core\Config\Configurable; use SilverStripe\Core\Injector\Injectable; use SilverStripe\Core\Injector\Injector; use SilverStripe\Core\Resettable; +use SilverStripe\ORM\FieldType\DBDatetime; class HTTPCacheControlMiddleware implements HTTPMiddleware, Resettable { @@ -43,14 +44,12 @@ class HTTPCacheControlMiddleware implements HTTPMiddleware, Resettable $response = $ex->getResponse(); } - // If sessions exist we assume that the responses should not be cached by CDNs / proxies as we are - // likely to be supplying information relevant to the current user only - if ($request->getSession()->getAll()) { - // Don't force in case user code chooses to opt in to public caching - $this->privateCache(); - } + // Update state based on current request and response objects + $this->augmentState($request, $response); + + // Add all headers to this response object + $this->applyToResponse($response); - HTTP::add_cache_headers($response); if (isset($ex)) { throw $ex; } @@ -87,12 +86,20 @@ class HTTPCacheControlMiddleware implements HTTPMiddleware, Resettable ], ]; + /** + * Set default state + * + * @config + * @var string + */ + protected static $defaultState = self::STATE_DISABLED; + /** * Current state * * @var string */ - protected $state = self::STATE_DISABLED; + protected $state = null; /** * Forcing level of previous setting; higher number wins @@ -100,7 +107,39 @@ class HTTPCacheControlMiddleware implements HTTPMiddleware, Resettable *w * @var int */ - protected $forcingLevel = 0; + protected $forcingLevel = null; + + /** + * List of vary keys + * + * @var array|null + */ + protected $vary = null; + + /** + * Latest modification date for this response + * + * @var int + */ + protected $modificationDate; + + /** + * Default vary + * + * @var array + */ + private static $defaultVary = [ + "X-Requested-With" => true, + "X-Forwarded-Protocol" => true, + ]; + + /** + * Default forcing level + * + * @config + * @var int + */ + private static $defaultForcingLevel = 0; /** * Forcing level forced, optionally combined with one of the below. @@ -148,6 +187,84 @@ class HTTPCacheControlMiddleware implements HTTPMiddleware, Resettable 'no-transform', ]; + /** + * Get current vary keys + * + * @return array + */ + public function getVary() + { + // Explicitly set vary + if (isset($this->vary)) { + return $this->vary; + } + + // Load default from config + $defaultVary = $this->config()->get('defaultVary'); + return array_keys(array_filter($defaultVary)); + } + + /** + * Add a vary + * + * @param string|array $vary + * @return $this + */ + public function addVary($vary) + { + $combied = $this->combineVary($this->getVary(), $vary); + $this->setVary($combied); + return $this; + } + + /** + * Set vary + * + * @param array|string $vary + * @return $this + */ + public function setVary($vary) + { + $this->vary = $this->combineVary($vary); + return $this; + } + + /** + * Combine vary strings/arrays into a single array, or normalise a single vary + * + * @param string|array[] $varies Each vary as a separate arg + * @return array + */ + protected function combineVary(...$varies) + { + $merged = []; + foreach ($varies as $vary) { + if ($vary && is_string($vary)) { + $vary = array_filter(preg_split("/\s*,\s*/", trim($vary))); + } + if ($vary && is_array($vary)) { + $merged = array_merge($merged, $vary); + } + } + return array_unique($merged); + } + + + /** + * Register a modification date. Used to calculate the Modification-Date http header + * + * @param string|int $date Date string or timestamp + * @return HTTPCacheControlMiddleware + */ + public function registerModificationDate($date) + { + $timestamp = is_numeric($date) ? $date : strtotime($date); + if ($timestamp > $this->modificationDate) { + $this->modificationDate = $timestamp; + } + return $this; + } + /** * Set current state. Should only be invoked internally after processing precedence rules. * @@ -170,7 +287,7 @@ class HTTPCacheControlMiddleware implements HTTPMiddleware, Resettable */ public function getState() { - return $this->state; + return $this->state ?: $this->config()->get('defaultState'); } /** @@ -190,7 +307,7 @@ class HTTPCacheControlMiddleware implements HTTPMiddleware, Resettable protected function applyChangeLevel($level, $force) { $forcingLevel = $level + ($force ? self::LEVEL_FORCED : 0); - if ($forcingLevel < $this->forcingLevel) { + if ($forcingLevel < $this->getForcingLevel()) { return false; } $this->forcingLevel = $forcingLevel; @@ -514,7 +631,7 @@ class HTTPCacheControlMiddleware implements HTTPMiddleware, Resettable */ public function applyToResponse($response) { - $headers = $this->generateHeaders(); + $headers = $this->generateHeadersFor($response); foreach ($headers as $name => $value) { $response->addHeader($name, $value); } @@ -542,13 +659,17 @@ class HTTPCacheControlMiddleware implements HTTPMiddleware, Resettable /** * Generate all headers to output * + * @param HTTPResponse $response * @return array */ - public function generateHeaders() + public function generateHeadersFor(HTTPResponse $response) { - return [ + return array_filter([ + 'Last-Modified' => $this->generateLastModifiedHeader(), + 'Vary' => $this->generateVaryHeader($response), 'Cache-Control' => $this->generateCacheHeader(), - ]; + 'Expires' => $this->generateExpiresHeader(), + ]); } /** @@ -558,4 +679,98 @@ class HTTPCacheControlMiddleware implements HTTPMiddleware, Resettable { Injector::inst()->unregisterNamedObject(__CLASS__); } + + /** + * @return int + */ + protected function getForcingLevel() + { + if (isset($this->forcingLevel)) { + return $this->forcingLevel; + } + return $this->config()->get('defaultForcingLevel'); + } + + /** + * Generate vary http header + * + * @param HTTPResponse $response + * @return string|null + */ + protected function generateVaryHeader(HTTPResponse $response) + { + // split the current vary header into it's parts and merge it with the config settings + // to create a list of unique vary values + $vary = $this->getVary(); + if ($response->getHeader('Vary')) { + $vary = $this->combineVary($vary, $response->getHeader('Vary')); + } + if ($vary) { + return implode(', ', $vary); + } + return null; + } + + /** + * Generate Last-Modified header + * + * @return string|null + */ + protected function generateLastModifiedHeader() + { + if (!$this->modificationDate) { + return null; + } + return gmdate('D, d M Y H:i:s', $this->modificationDate) . ' GMT'; + } + + /** + * Generate Expires http header + * + * @return null|string + */ + protected function generateExpiresHeader() + { + $maxAge = $this->getDirective('max-age'); + if ($maxAge === false) { + return null; + } + + // Add now to max-age to generate expires + $expires = DBDatetime::now()->getTimestamp() + $maxAge; + return gmdate('D, d M Y H:i:s', $expires) . ' GMT'; + } + + /** + * Update state based on current request and response objects + * + * @param HTTPRequest $request + * @param HTTPResponse $response + */ + protected function augmentState(HTTPRequest $request, HTTPResponse $response) + { + // If sessions exist we assume that the responses should not be cached by CDNs / proxies as we are + // likely to be supplying information relevant to the current user only + if ($request->getSession()->getAll()) { + // Don't force in case user code chooses to opt in to public caching + $this->privateCache(); + } + + // IE6-IE8 have problems saving files when https and no-cache/no-store are used + // (http://support.microsoft.com/kb/323308) + // Note: this is also fixable by ticking "Do not save encrypted pages to disk" in advanced options. + if ($request->getScheme() === 'https' && + preg_match('/.+MSIE (7|8).+/', $request->getHeader('User-Agent')) && + strstr($response->getHeader('Content-Disposition'), 'attachment;') == true && + ($this->hasDirective('no-cache') || $this->hasDirective('no-store')) + ) { + $this->privateCache(true); + } + + // Errors disable cache (unless some errors are cached intentionally by usercode) + if ($response->isError()) { + // Even if publicCache(true) is specfied, errors will be uncachable + $this->disableCache(true); + } + } } diff --git a/src/ORM/DataObject.php b/src/ORM/DataObject.php index a72c55879..04edcb964 100644 --- a/src/ORM/DataObject.php +++ b/src/ORM/DataObject.php @@ -7,6 +7,7 @@ use Exception; use InvalidArgumentException; use LogicException; use SilverStripe\Control\HTTP; +use SilverStripe\Control\Middleware\HTTPCacheControlMiddleware; use SilverStripe\Core\ClassInfo; use SilverStripe\Core\Config\Config; use SilverStripe\Core\Injector\Injector; @@ -382,7 +383,8 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity // Keep track of the modification date of all the data sourced to make this page // From this we create a Last-Modified HTTP header if (isset($record['LastEdited'])) { - HTTP::register_modification_date($record['LastEdited']); + HTTPCacheControlMiddleware::singleton() + ->registerModificationDate($record['LastEdited']); } // Must be called after parent constructor diff --git a/tests/php/Control/HTTPCacheControlIntegrationTest.php b/tests/php/Control/HTTPCacheControlIntegrationTest.php index 29af2024e..94b0f1527 100644 --- a/tests/php/Control/HTTPCacheControlIntegrationTest.php +++ b/tests/php/Control/HTTPCacheControlIntegrationTest.php @@ -19,7 +19,9 @@ class HTTPCacheControlIntegrationTest extends FunctionalTest protected function setUp() { parent::setUp(); - Config::modify()->remove(HTTP::class, 'disable_http_cache'); + HTTPCacheControlMiddleware::config() + ->set('defaultState', 'disabled') + ->set('defaultForcingLevel', 0); HTTPCacheControlMiddleware::reset(); } diff --git a/tests/php/Control/HTTPTest.php b/tests/php/Control/HTTPTest.php index 7de7966ee..ddefc2991 100644 --- a/tests/php/Control/HTTPTest.php +++ b/tests/php/Control/HTTPTest.php @@ -22,8 +22,10 @@ class HTTPTest extends FunctionalTest protected function setUp() { parent::setUp(); - // Remove dev-only config - Config::modify()->remove(HTTP::class, 'disable_http_cache'); + // Set to disabled at null forcing level + HTTPCacheControlMiddleware::config() + ->set('defaultState', 'disabled') + ->set('defaultForcingLevel', 0); HTTPCacheControlMiddleware::reset(); } @@ -38,7 +40,9 @@ class HTTPTest extends FunctionalTest $this->assertNotEmpty($response->getHeader('Cache-Control')); // Ensure cache headers are set correctly when disabled via config (e.g. when dev) - Config::modify()->set(HTTP::class, 'disable_http_cache', true); + HTTPCacheControlMiddleware::config() + ->set('defaultState', 'disabled') + ->set('defaultForcingLevel', HTTPCacheControlMiddleware::LEVEL_DISABLED); HTTPCacheControlMiddleware::reset(); HTTPCacheControlMiddleware::singleton()->publicCache(); HTTPCacheControlMiddleware::singleton()->setMaxAge(30); @@ -49,7 +53,9 @@ class HTTPTest extends FunctionalTest $this->assertContains('must-revalidate', $response->getHeader('Cache-Control')); // Ensure max-age setting is respected in production. - Config::modify()->remove(HTTP::class, 'disable_http_cache'); + HTTPCacheControlMiddleware::config() + ->set('defaultState', 'disabled') + ->set('defaultForcingLevel', 0); HTTPCacheControlMiddleware::reset(); HTTPCacheControlMiddleware::singleton()->publicCache(); HTTPCacheControlMiddleware::singleton()->setMaxAge(30); diff --git a/tests/php/Control/Middleware/HTTPCacheControlMiddlewareTest.php b/tests/php/Control/Middleware/HTTPCacheControlMiddlewareTest.php index af4e1a23f..3eeff44f0 100644 --- a/tests/php/Control/Middleware/HTTPCacheControlMiddlewareTest.php +++ b/tests/php/Control/Middleware/HTTPCacheControlMiddlewareTest.php @@ -7,6 +7,16 @@ use SilverStripe\Dev\SapphireTest; class HTTPCacheControlMiddlewareTest extends SapphireTest { + protected function setUp() + { + parent::setUp(); + // Set to disabled at null forcing level + HTTPCacheControlMiddleware::config() + ->set('defaultState', 'disabled') + ->set('defaultForcingLevel', 0); + HTTPCacheControlMiddleware::reset(); + } + public function testCachingPriorities() { $hcc = new HTTPCacheControlMiddleware(); From 0b308c871d1a757d21e9ec40a6ebe4899693d2e8 Mon Sep 17 00:00:00 2001 From: Daniel Hensby Date: Wed, 13 Jun 2018 14:29:10 +0100 Subject: [PATCH 13/34] DOCS Update doc errors --- .../08_Performance/02_HTTP_Cache_Headers.md | 16 ++++++++-------- docs/en/04_Changelogs/4.2.0.md | 10 +++++----- .../Middleware/HTTPCacheControlMiddleware.php | 2 +- 3 files changed, 14 insertions(+), 14 deletions(-) diff --git a/docs/en/02_Developer_Guides/08_Performance/02_HTTP_Cache_Headers.md b/docs/en/02_Developer_Guides/08_Performance/02_HTTP_Cache_Headers.md index 946ec67a7..339b17fef 100644 --- a/docs/en/02_Developer_Guides/08_Performance/02_HTTP_Cache_Headers.md +++ b/docs/en/02_Developer_Guides/08_Performance/02_HTTP_Cache_Headers.md @@ -20,11 +20,11 @@ are a great way to learn about HTTP caching. ### Overview In order to support developers in making safe choices around HTTP caching, -we're using a `HTTPCacheControl` class to control if a response +we're using a `HTTPCacheControlMiddleware` class to control if a response should be considered public or private. This is an abstraction on existing -lowlevel APIs like `HTTP::add_cache_headers()` and `SS_HTTPResponse->addHeader()`. +lowlevel APIs like `HTTP::add_cache_headers()` and `HTTPResponse->addHeader()`. -The `HTTPCacheControl` API makes it easier to express your caching preferences +The `HTTPCacheControlMiddleware` API makes it easier to express your caching preferences without running the risk of overriding essential core safety measures. Most commonly, these APIs will prevent HTTP caching of draft content. @@ -96,7 +96,7 @@ The priority order is as followed, sorted in descending order ### Global opt-in for page content -Enable caching for all page content (through `Page_Controller`). +Enable caching for all page content (through `PageController`). ```php disableCache(); - return $this->myPrivateResponse();; + return $this->myPrivateResponse(); } } ``` @@ -173,7 +173,7 @@ class PageController extends ContentController { public function init() { - HTTPCacheControlMiddleware::inst() + HTTPCacheControlMiddleware::singleton() ->enableCache($force=true) // DANGER ZONE ->setMaxAge(60); // 1 minute @@ -199,7 +199,7 @@ headers: The cache age determines the lifetime of your cache, in seconds. It only takes effect if you instruct the cache control -that your response is public in the first place (via `enableCache()` or via modifying the `HTTP.cache_control` defaults). +that your response is cacheable in the first place (via `enableCache()` or via modifying the `HTTP.cache_control` defaults). ```php use SilverStripe\Control\Middleware\HTTPCacheControlMiddleware; @@ -222,7 +222,7 @@ HTTP::register_modification_date('2014-10-10'); ### Vary A `Vary` header tells caches which aspects of the response should be considered -when calculating a cache key, usually in addition to the full URL path. +when calculating a cache key, usually in addition to the full URL. By default, SilverStripe will output a `Vary` header with the following content: ``` diff --git a/docs/en/04_Changelogs/4.2.0.md b/docs/en/04_Changelogs/4.2.0.md index 090975968..807985480 100644 --- a/docs/en/04_Changelogs/4.2.0.md +++ b/docs/en/04_Changelogs/4.2.0.md @@ -209,7 +209,7 @@ SilverStripe\Core\Injector\Injector: In order to support developers in making safe choices around HTTP caching, we've introduced a `HTTPCacheControlMiddleware` class to control if a response should be considered public or private. This is an abstraction on existing -lowlevel APIs like `HTTP::add_cache_headers()` and `SS_HTTPResponse->addHeader()`. +lowlevel APIs like `HTTP::add_cache_headers()` and `HTTPResponse->addHeader()`. This change introduces smaller but necessary changes to HTTP caching headers sent by SilverStripe. If you are relying on HTTP caching in your implementation, @@ -240,7 +240,7 @@ for details on the new API. ##### Global opt-in for page content -Enable caching for all page content (through `Page_Controller`). +Enable caching for all page content (through `PageController`). ```diff enableCache() + ->setMaxAge(60); // 1 minute @@ -286,7 +286,7 @@ class PageController extends ContentController public function myprivateaction($request) { - HTTP::set_cache_age(0); -+ HTTPCacheControl::inst() ++ HTTPCacheControlMiddleware::singleton() + ->disableCache(); return $this->myPrivateResponse(); @@ -323,7 +323,7 @@ class PageController extends ContentController public function init() { - HTTP::set_cache_age(60); -+ HTTPCacheControl::inst() ++ HTTPCacheControlMiddleware::singleton() + ->enableCache($force=true) // DANGER ZONE + ->setMaxAge(60); // 1 minute diff --git a/src/Control/Middleware/HTTPCacheControlMiddleware.php b/src/Control/Middleware/HTTPCacheControlMiddleware.php index 78aa2540a..e849d3d0d 100644 --- a/src/Control/Middleware/HTTPCacheControlMiddleware.php +++ b/src/Control/Middleware/HTTPCacheControlMiddleware.php @@ -769,7 +769,7 @@ class HTTPCacheControlMiddleware implements HTTPMiddleware, Resettable // Errors disable cache (unless some errors are cached intentionally by usercode) if ($response->isError()) { - // Even if publicCache(true) is specfied, errors will be uncachable + // Even if publicCache(true) is specified, errors will be uncacheable $this->disableCache(true); } } From 17ad9859254285ddd212f398508965cc9907bbac Mon Sep 17 00:00:00 2001 From: Daniel Hensby Date: Wed, 13 Jun 2018 14:29:37 +0100 Subject: [PATCH 14/34] Cleanup ETag middleware --- src/Control/Middleware/ETagMiddleware.php | 21 ++++----------------- 1 file changed, 4 insertions(+), 17 deletions(-) diff --git a/src/Control/Middleware/ETagMiddleware.php b/src/Control/Middleware/ETagMiddleware.php index cfc360340..b7cfabc7a 100644 --- a/src/Control/Middleware/ETagMiddleware.php +++ b/src/Control/Middleware/ETagMiddleware.php @@ -8,13 +8,6 @@ use SilverStripe\Core\Injector\Injectable; /** * Generates and handle responses for etag header. - * - * Chrome ignores Varies when redirecting back (http://code.google.com/p/chromium/issues/detail?id=79758) - * which means that if you log out, you get redirected back to a page which Chrome then checks against - * last-modified (which passes, getting a 304) - * when it shouldn't be trying to use that page at all because it's the "logged in" version. - * By also using and etag that includes both the modification date and all the varies - * values which we also check against we can catch this and not return a 304 */ class ETagMiddleware implements HTTPMiddleware { @@ -45,7 +38,7 @@ class ETagMiddleware implements HTTPMiddleware // Check if we have a match $ifNoneMatch = $request->getHeader('If-None-Match'); - if ($ifNoneMatch && $ifNoneMatch === $etag) { + if ($ifNoneMatch === $etag) { return $this->sendNotModified($request, $response); } } @@ -67,18 +60,12 @@ class ETagMiddleware implements HTTPMiddleware protected function generateETag(HTTPResponse $response) { // Existing e-tag - if ($response instanceof HTTPResponse && $response->getHeader('ETag')) { - return $response->getHeader('ETag'); + if ($etag = $response->getHeader('ETag')) { + return $etag; } // Generate etag from body - $body = $response instanceof HTTPResponse - ? $response->getBody() - : $response; - if ($body) { - return sprintf('"%s"', md5($body)); - } - return false; + return sprintf('"%s"', md5($response->getBody())); } /** From 1b425570cf06a8cfd5535afaa4a26e8cfda3d6ca Mon Sep 17 00:00:00 2001 From: Daniel Hensby Date: Wed, 13 Jun 2018 15:27:20 +0100 Subject: [PATCH 15/34] Remove IE edge case handling --- src/Control/Middleware/HTTPCacheControlMiddleware.php | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/src/Control/Middleware/HTTPCacheControlMiddleware.php b/src/Control/Middleware/HTTPCacheControlMiddleware.php index e849d3d0d..839ac28e0 100644 --- a/src/Control/Middleware/HTTPCacheControlMiddleware.php +++ b/src/Control/Middleware/HTTPCacheControlMiddleware.php @@ -756,17 +756,6 @@ class HTTPCacheControlMiddleware implements HTTPMiddleware, Resettable $this->privateCache(); } - // IE6-IE8 have problems saving files when https and no-cache/no-store are used - // (http://support.microsoft.com/kb/323308) - // Note: this is also fixable by ticking "Do not save encrypted pages to disk" in advanced options. - if ($request->getScheme() === 'https' && - preg_match('/.+MSIE (7|8).+/', $request->getHeader('User-Agent')) && - strstr($response->getHeader('Content-Disposition'), 'attachment;') == true && - ($this->hasDirective('no-cache') || $this->hasDirective('no-store')) - ) { - $this->privateCache(true); - } - // Errors disable cache (unless some errors are cached intentionally by usercode) if ($response->isError()) { // Even if publicCache(true) is specified, errors will be uncacheable From a88257efac1a222607d413c070ab4bd7dc20df3d Mon Sep 17 00:00:00 2001 From: Daniel Hensby Date: Wed, 13 Jun 2018 15:28:37 +0100 Subject: [PATCH 16/34] NEW Add version to HTTPRequest and create raw string representation --- src/Control/HTTPResponse.php | 86 ++++++++++++++++++++++++++++++------ 1 file changed, 72 insertions(+), 14 deletions(-) diff --git a/src/Control/HTTPResponse.php b/src/Control/HTTPResponse.php index 987162e65..e73b97bf2 100644 --- a/src/Control/HTTPResponse.php +++ b/src/Control/HTTPResponse.php @@ -19,7 +19,7 @@ class HTTPResponse /** * @var array */ - protected static $status_codes = array( + protected static $status_codes = [ 100 => 'Continue', 101 => 'Switching Protocols', 200 => 'OK', @@ -61,20 +61,25 @@ class HTTPResponse 503 => 'Service Unavailable', 504 => 'Gateway Timeout', 505 => 'HTTP Version Not Supported', - ); + ]; /** * @var array */ - protected static $redirect_codes = array( + protected static $redirect_codes = [ 301, 302, 303, 304, 305, 307, - 308 - ); + 308, + ]; + + /** + * @var string + */ + protected $version = '1.0'; /** * @var int @@ -92,9 +97,9 @@ class HTTPResponse * @see http://en.wikipedia.org/wiki/List_of_HTTP_headers * @var array */ - protected $headers = array( + protected $headers = [ "content-type" => "text/html; charset=utf-8", - ); + ]; /** * @var string @@ -109,12 +114,31 @@ class HTTPResponse * @param string $statusDescription The text to be given alongside the status code. * See {@link setStatusCode()} for more information. */ - public function __construct($body = null, $statusCode = null, $statusDescription = null) + public function __construct($body = null, $statusCode = null, $statusDescription = null, $version = null) { $this->setBody($body); if ($statusCode) { $this->setStatusCode($statusCode, $statusDescription); } + if (!$version) { + if (preg_match('/HTTP\/(\d+(\.\d+)?)/i', $_SERVER['SERVER_PROTOCOL'], $matches)) { + $version = $matches[1]; + } + } + $this->setVersion($version); + } + + /** + * The HTTP version used to respond to this request (typically 1.0 or 1.1) + * + * @param string $version + * + * @return $this + */ + public function setVersion($version) + { + $this->version = $version; + return $this; } /** @@ -123,6 +147,7 @@ class HTTPResponse * No newlines are allowed in the description. * If omitted, will default to the standard HTTP description * for the given $code value (see {@link $status_codes}). + * * @return $this */ public function setStatusCode($code, $description = null) @@ -146,6 +171,7 @@ class HTTPResponse * Caution: Will be overwritten by {@link setStatusCode()}. * * @param string $description + * * @return $this */ public function setStatusDescription($description) @@ -154,6 +180,14 @@ class HTTPResponse return $this; } + /** + * @return string + */ + public function getVersion() + { + return $this->version; + } + /** * @return int */ @@ -167,7 +201,7 @@ class HTTPResponse */ public function getStatusDescription() { - return str_replace(array("\r","\n"), '', $this->statusDescription); + return str_replace(["\r", "\n"], '', $this->statusDescription); } /** @@ -183,11 +217,12 @@ class HTTPResponse /** * @param string $body + * * @return $this */ public function setBody($body) { - $this->body = $body ? (string) $body : $body; // Don't type-cast false-ish values, eg null is null not '' + $this->body = $body ? (string)$body : $body; // Don't type-cast false-ish values, eg null is null not '' return $this; } @@ -204,6 +239,7 @@ class HTTPResponse * * @param string $header Example: "content-type" * @param string $value Example: "text/xml" + * * @return $this */ public function addHeader($header, $value) @@ -217,6 +253,7 @@ class HTTPResponse * Return the HTTP header of the given name. * * @param string $header + * * @return string */ public function getHeader($header) @@ -241,6 +278,7 @@ class HTTPResponse * e.g. "Content-Type". * * @param string $header + * * @return $this */ public function removeHeader($header) @@ -253,6 +291,7 @@ class HTTPResponse /** * @param string $dest * @param int $code + * * @return $this */ public function redirect($dest, $code = 302) @@ -322,7 +361,7 @@ EOT ); header($method); foreach ($this->getHeaders() as $header => $value) { - header("{$header}: {$value}", true, $this->getStatusCode()); + header("{$header}: {$value}", true, $this->getStatusCode()); } } elseif ($this->getStatusCode() >= 300) { // It's critical that these status codes are sent; we need to report a failure if not. @@ -351,9 +390,9 @@ EOT /** @var HandlerInterface $handler */ $handler = Injector::inst()->get(HandlerInterface::class); $formatter = $handler->getFormatter(); - echo $formatter->format(array( - 'code' => $this->statusCode - )); + echo $formatter->format([ + 'code' => $this->statusCode, + ]); } else { echo $this->body; } @@ -379,4 +418,23 @@ EOT { return in_array($this->getStatusCode(), self::$redirect_codes); } + + /** + * The HTTP response represented as a raw string + * + * @return string + */ + public function __toString() + { + $headers = []; + foreach ($this->getHeaders() as $header => $values) { + foreach ((array)$values as $value) { + $headers[] = sprintf('%s: %s', $header, $value); + } + } + return + sprintf('HTTP/%s %s %s', $this->getVersion(), $this->getStatusCode(), $this->getStatusDescription()) . "\r\n" . + implode("\r\n", $headers) . "\r\n" . "\r\n" . + $this->getBody(); + } } From 6008457f3e15f15d4aa7b836c1ef057034a58e64 Mon Sep 17 00:00:00 2001 From: Ingo Schommer Date: Thu, 14 Jun 2018 10:30:01 +1200 Subject: [PATCH 17/34] Clearer deprecation of HTTP in docs --- .../08_Performance/02_HTTP_Cache_Headers.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/docs/en/02_Developer_Guides/08_Performance/02_HTTP_Cache_Headers.md b/docs/en/02_Developer_Guides/08_Performance/02_HTTP_Cache_Headers.md index 339b17fef..232bc350f 100644 --- a/docs/en/02_Developer_Guides/08_Performance/02_HTTP_Cache_Headers.md +++ b/docs/en/02_Developer_Guides/08_Performance/02_HTTP_Cache_Headers.md @@ -38,7 +38,8 @@ since there are too many variations under which output could be considered priva (e.g. a custom "approval" flag on a comment object). It is up to the developer to ensure caching is used appropriately there. -The [api:SilverStripe\Control\Middleware\HTTPCacheControlMiddleware] class supplements the `HTTP` helper class. +The [api:SilverStripe\Control\Middleware\HTTPCacheControlMiddleware] class replaces +(deprecated) caching methods in the `HTTP` helper class. It comes with methods which let developers safely interact with the `Cache-Control` header. ### disableCache() From 015b897d8260466976065d6a0a2144d358ef1935 Mon Sep 17 00:00:00 2001 From: Ingo Schommer Date: Thu, 14 Jun 2018 10:30:13 +1200 Subject: [PATCH 18/34] Link to caching docs from "form security" docs --- docs/en/02_Developer_Guides/03_Forms/04_Form_Security.md | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/en/02_Developer_Guides/03_Forms/04_Form_Security.md b/docs/en/02_Developer_Guides/03_Forms/04_Form_Security.md index a34971a60..82d270c9d 100644 --- a/docs/en/02_Developer_Guides/03_Forms/04_Form_Security.md +++ b/docs/en/02_Developer_Guides/03_Forms/04_Form_Security.md @@ -86,6 +86,7 @@ accessible publicly through the CDN. To ensure this doesn't happen SilverStripe adds `Cache-Control: no-store, no-cache, must-revalidate` headers to any forms that have validators or security tokens (all of them by default) applied to them; this ensures that CDNs (and browsers) will not cache these pages. +See [/developer_guides/performance/http_cache_headers](Performance: HTTP Cache Headers). ## Related Documentation From b7e54bad249df3d5abab4a5d68cd35cad44934d8 Mon Sep 17 00:00:00 2001 From: Damian Mooyman Date: Thu, 14 Jun 2018 11:04:07 +1200 Subject: [PATCH 19/34] Adjust HTTPResponse::getVersion() to match PSR-7 Method signature --- src/Control/HTTPResponse.php | 27 +++++++++++++---------- src/Control/Middleware/ETagMiddleware.php | 3 ++- 2 files changed, 17 insertions(+), 13 deletions(-) diff --git a/src/Control/HTTPResponse.php b/src/Control/HTTPResponse.php index e73b97bf2..3ec7006c4 100644 --- a/src/Control/HTTPResponse.php +++ b/src/Control/HTTPResponse.php @@ -79,7 +79,7 @@ class HTTPResponse /** * @var string */ - protected $version = '1.0'; + protected $protocolVersion = '1.0'; /** * @var int @@ -113,31 +113,34 @@ class HTTPResponse * @param int $statusCode The numeric status code - 200, 404, etc * @param string $statusDescription The text to be given alongside the status code. * See {@link setStatusCode()} for more information. + * @param string $protocolVersion */ - public function __construct($body = null, $statusCode = null, $statusDescription = null, $version = null) + public function __construct($body = null, $statusCode = null, $statusDescription = null, $protocolVersion = null) { $this->setBody($body); if ($statusCode) { $this->setStatusCode($statusCode, $statusDescription); } - if (!$version) { - if (preg_match('/HTTP\/(\d+(\.\d+)?)/i', $_SERVER['SERVER_PROTOCOL'], $matches)) { - $version = $matches[1]; + if (!$protocolVersion) { + if (preg_match('/HTTP\/(?\d+(\.\d+)?)/i', $_SERVER['SERVER_PROTOCOL'], $matches)) { + $protocolVersion = $matches['version']; } } - $this->setVersion($version); + if ($protocolVersion) { + $this->setProtocolVersion($protocolVersion); + } } /** * The HTTP version used to respond to this request (typically 1.0 or 1.1) * - * @param string $version + * @param string $protocolVersion * * @return $this */ - public function setVersion($version) + public function setProtocolVersion($protocolVersion) { - $this->version = $version; + $this->protocolVersion = $protocolVersion; return $this; } @@ -183,9 +186,9 @@ class HTTPResponse /** * @return string */ - public function getVersion() + public function getProtocolVersion() { - return $this->version; + return $this->protocolVersion; } /** @@ -433,7 +436,7 @@ EOT } } return - sprintf('HTTP/%s %s %s', $this->getVersion(), $this->getStatusCode(), $this->getStatusDescription()) . "\r\n" . + sprintf('HTTP/%s %s %s', $this->getProtocolVersion(), $this->getStatusCode(), $this->getStatusDescription()) . "\r\n" . implode("\r\n", $headers) . "\r\n" . "\r\n" . $this->getBody(); } diff --git a/src/Control/Middleware/ETagMiddleware.php b/src/Control/Middleware/ETagMiddleware.php index b7cfabc7a..992040634 100644 --- a/src/Control/Middleware/ETagMiddleware.php +++ b/src/Control/Middleware/ETagMiddleware.php @@ -60,7 +60,8 @@ class ETagMiddleware implements HTTPMiddleware protected function generateETag(HTTPResponse $response) { // Existing e-tag - if ($etag = $response->getHeader('ETag')) { + $etag = $response->getHeader('ETag'); + if ($etag) { return $etag; } From 6b68495c0d39ef7d916e4905b9de1a842055654c Mon Sep 17 00:00:00 2001 From: Damian Mooyman Date: Thu, 14 Jun 2018 11:16:52 +1200 Subject: [PATCH 20/34] Rename ETagMiddleware to ChangeDetectionMiddleware --- _config/requestprocessors.yml | 2 +- src/Control/HTTP.php | 12 ++++++------ ...gMiddleware.php => ChangeDetectionMiddleware.php} | 5 +++-- 3 files changed, 10 insertions(+), 9 deletions(-) rename src/Control/Middleware/{ETagMiddleware.php => ChangeDetectionMiddleware.php} (92%) diff --git a/_config/requestprocessors.yml b/_config/requestprocessors.yml index 46917c8db..13cb7cdf6 100644 --- a/_config/requestprocessors.yml +++ b/_config/requestprocessors.yml @@ -11,7 +11,7 @@ SilverStripe\Core\Injector\Injector: SessionMiddleware: '%$SilverStripe\Control\Middleware\SessionMiddleware' RequestProcessorMiddleware: '%$SilverStripe\Control\RequestProcessor' FlushMiddleware: '%$SilverStripe\Control\Middleware\FlushMiddleware' - ETagMiddleware: '%$SilverStripe\Control\Middleware\ETagMiddleware' + ChangeDetectionMiddleware: '%$SilverStripe\Control\Middleware\ChangeDetectionMiddleware' HTTPCacheControleMiddleware: '%$SilverStripe\Control\Middleware\HTTPCacheControlMiddleware' CanonicalURLMiddleware: '%$SilverStripe\Control\Middleware\CanonicalURLMiddleware' SilverStripe\Control\Middleware\AllowedHostsMiddleware: diff --git a/src/Control/HTTP.php b/src/Control/HTTP.php index 0277e9ed6..d50a82091 100644 --- a/src/Control/HTTP.php +++ b/src/Control/HTTP.php @@ -3,7 +3,7 @@ namespace SilverStripe\Control; use SilverStripe\Assets\File; -use SilverStripe\Control\Middleware\ETagMiddleware; +use SilverStripe\Control\Middleware\ChangeDetectionMiddleware; use SilverStripe\Control\Middleware\HTTPCacheControlMiddleware; use SilverStripe\Core\Config\Config; use SilverStripe\Core\Config\Configurable; @@ -33,7 +33,7 @@ class HTTP protected static $modification_date = null; /** - * @deprecated 4.2..5.0 Handled by ETagMiddleware + * @deprecated 4.2..5.0 Handled by ChangeDetectionMiddleware * @var string */ protected static $etag = null; @@ -395,12 +395,12 @@ class HTTP } /** - * @deprecated 4.2..5.0 Use ETagMiddleware instead + * @deprecated 4.2..5.0 Use ChangeDetectionMiddleware instead * @param string $etag */ public static function register_etag($etag) { - Deprecation::notice('5.0', 'Use ETagMiddleware instead'); + Deprecation::notice('5.0', 'Use ChangeDetectionMiddleware instead'); if (strpos($etag, '"') !== 0) { $etag = "\"{$etag}\""; } @@ -453,7 +453,7 @@ class HTTP // Get current cache control state $cacheControlMiddleware = HTTPCacheControlMiddleware::singleton(); - $etagMiddleware = ETagMiddleware::singleton(); + $changeDetectionMiddleware = ChangeDetectionMiddleware::singleton(); // if http caching is disabled by config, disable it - used on dev environments due to frequently changing // templates and other data. will be overridden by forced publicCache(true) or privateCache(true) calls @@ -491,7 +491,7 @@ class HTTP } // Run middleware - $etagMiddleware->process($request, function (HTTPRequest $request) use ($cacheControlMiddleware, $response) { + $changeDetectionMiddleware->process($request, function (HTTPRequest $request) use ($cacheControlMiddleware, $response) { return $cacheControlMiddleware->process($request, function (HTTPRequest $request) use ($response) { return $response; }); diff --git a/src/Control/Middleware/ETagMiddleware.php b/src/Control/Middleware/ChangeDetectionMiddleware.php similarity index 92% rename from src/Control/Middleware/ETagMiddleware.php rename to src/Control/Middleware/ChangeDetectionMiddleware.php index 992040634..833483134 100644 --- a/src/Control/Middleware/ETagMiddleware.php +++ b/src/Control/Middleware/ChangeDetectionMiddleware.php @@ -7,9 +7,10 @@ use SilverStripe\Control\HTTPResponse; use SilverStripe\Core\Injector\Injectable; /** - * Generates and handle responses for etag header. + * Handles internal change detection via etag / ifmodifiedsince headers, + * conditonally sending a 304 not modified if possible. */ -class ETagMiddleware implements HTTPMiddleware +class ChangeDetectionMiddleware implements HTTPMiddleware { use Injectable; From e93289326eb3d951a1ff81402b63f9d31482005b Mon Sep 17 00:00:00 2001 From: Damian Mooyman Date: Thu, 14 Jun 2018 11:17:21 +1200 Subject: [PATCH 21/34] Add Form::canBeCached() method --- src/Forms/Form.php | 54 +++++++++++++++++++++++++++++----------------- 1 file changed, 34 insertions(+), 20 deletions(-) diff --git a/src/Forms/Form.php b/src/Forms/Form.php index b2d921a34..186ebf659 100644 --- a/src/Forms/Form.php +++ b/src/Forms/Form.php @@ -5,7 +5,6 @@ namespace SilverStripe\Forms; use BadMethodCallException; use SilverStripe\Control\Controller; use SilverStripe\Control\HasRequestHandler; -use SilverStripe\Control\HTTP; use SilverStripe\Control\HTTPRequest; use SilverStripe\Control\Middleware\HTTPCacheControlMiddleware; use SilverStripe\Control\NullHTTPRequest; @@ -869,25 +868,6 @@ class Form extends ViewableData implements HasRequestHandler { $exclude = (is_string($attrs)) ? func_get_args() : null; - // Figure out if we can cache this form - // - forms with validation shouldn't be cached, cos their error messages won't be shown - // - forms with security tokens shouldn't be cached because security tokens expire - $needsCacheDisabled = false; - if ($this->getSecurityToken()->isEnabled()) { - $needsCacheDisabled = true; - } - if ($this->FormMethod() != 'GET') { - $needsCacheDisabled = true; - } - if (!($this->validator instanceof RequiredFields) || count($this->validator->getRequired())) { - $needsCacheDisabled = true; - } - - // If we need to disable cache, do it - if ($needsCacheDisabled) { - HTTPCacheControlMiddleware::singleton()->disableCache(); - } - $attrs = $this->getAttributes(); // Remove empty @@ -1569,6 +1549,10 @@ class Form extends ViewableData implements HasRequestHandler */ public function forTemplate() { + if (!$this->canBeCached()) { + HTTPCacheControlMiddleware::singleton()->disableCache(); + } + $return = $this->renderWith($this->getTemplates()); // Now that we're rendered, clear message @@ -1824,4 +1808,34 @@ class Form extends ViewableData implements HasRequestHandler { return FormRequestHandler::create($this); } + + /** + * Can the body of this form be cached? + * + * Figure out if we can cache this form + * - forms with validation shouldn't be cached, because their error messages won't be shown + * - forms with security tokens shouldn't be cached because security tokens expire + * + * @return bool + */ + public function canBeCached() + { + if ($this->getSecurityToken()->isEnabled()) { + return false; + } + if ($this->FormMethod() !== 'GET') { + return false; + } + + // Don't cache if there are required fields, or some other complex validator + $validator = $this->getValidator(); + if ($validator instanceof RequiredFields) { + if (count($this->validator->getRequired())) { + return false; + } + } else { + return false; + } + return true; + } } From 59ba208df010e6a26afeed8c650a5b705ea9361b Mon Sep 17 00:00:00 2001 From: Damian Mooyman Date: Thu, 14 Jun 2018 11:46:28 +1200 Subject: [PATCH 22/34] Fix HTTPTest --- .../Middleware/HTTPCacheControlMiddleware.php | 6 +- tests/php/Control/HTTPTest.php | 64 ++++++++++++------- 2 files changed, 45 insertions(+), 25 deletions(-) diff --git a/src/Control/Middleware/HTTPCacheControlMiddleware.php b/src/Control/Middleware/HTTPCacheControlMiddleware.php index 839ac28e0..c91d9c641 100644 --- a/src/Control/Middleware/HTTPCacheControlMiddleware.php +++ b/src/Control/Middleware/HTTPCacheControlMiddleware.php @@ -623,7 +623,7 @@ class HTTPCacheControlMiddleware implements HTTPMiddleware, Resettable } /** - * Generate and add the `Cache-Control` header to a response object + * Generate all headers to add to this object * * @param HTTPResponse $response * @@ -633,7 +633,9 @@ class HTTPCacheControlMiddleware implements HTTPMiddleware, Resettable { $headers = $this->generateHeadersFor($response); foreach ($headers as $name => $value) { - $response->addHeader($name, $value); + if (!$response->getHeader($name)) { + $response->addHeader($name, $value); + } } return $this; } diff --git a/tests/php/Control/HTTPTest.php b/tests/php/Control/HTTPTest.php index ddefc2991..052186ca1 100644 --- a/tests/php/Control/HTTPTest.php +++ b/tests/php/Control/HTTPTest.php @@ -2,14 +2,13 @@ namespace SilverStripe\Control\Tests; -use PHPUnit_Framework_Error_Warning; use SilverStripe\Control\Controller; use SilverStripe\Control\Director; use SilverStripe\Control\HTTP; use SilverStripe\Control\HTTPRequest; use SilverStripe\Control\HTTPResponse; use SilverStripe\Control\Middleware\HTTPCacheControlMiddleware; -use SilverStripe\Core\Config\Config; +use SilverStripe\Control\Session; use SilverStripe\Dev\FunctionalTest; /** @@ -36,7 +35,7 @@ class HTTPTest extends FunctionalTest HTTPCacheControlMiddleware::singleton()->publicCache(); HTTPCacheControlMiddleware::singleton()->setMaxAge(30); - HTTP::add_cache_headers($response); + $this->addCacheHeaders($response); $this->assertNotEmpty($response->getHeader('Cache-Control')); // Ensure cache headers are set correctly when disabled via config (e.g. when dev) @@ -47,7 +46,7 @@ class HTTPTest extends FunctionalTest HTTPCacheControlMiddleware::singleton()->publicCache(); HTTPCacheControlMiddleware::singleton()->setMaxAge(30); $response = new HTTPResponse($body, 200); - HTTP::add_cache_headers($response); + $this->addCacheHeaders($response); $this->assertContains('no-cache', $response->getHeader('Cache-Control')); $this->assertContains('no-store', $response->getHeader('Cache-Control')); $this->assertContains('must-revalidate', $response->getHeader('Cache-Control')); @@ -60,7 +59,7 @@ class HTTPTest extends FunctionalTest HTTPCacheControlMiddleware::singleton()->publicCache(); HTTPCacheControlMiddleware::singleton()->setMaxAge(30); $response = new HTTPResponse($body, 200); - HTTP::add_cache_headers($response); + $this->addCacheHeaders($response); $this->assertContains('max-age=30', $response->getHeader('Cache-Control')); $this->assertNotContains('max-age=0', $response->getHeader('Cache-Control')); @@ -70,45 +69,45 @@ class HTTPTest extends FunctionalTest 'Pragma' => 'no-cache', 'Cache-Control' => 'max-age=0, no-cache, no-store', ); + foreach ($headers as $header => $value) { + $response->addHeader($header, $value); + } HTTPCacheControlMiddleware::reset(); HTTPCacheControlMiddleware::singleton()->publicCache(); HTTPCacheControlMiddleware::singleton()->setMaxAge(30); - $response = new HTTPResponse($body, 200); - foreach ($headers as $name => $value) { - $response->addHeader($name, $value); + $this->addCacheHeaders($response); + foreach ($headers as $header => $value) { + $this->assertEquals($value, $response->getHeader($header)); } - // Expect a warning if the header is already set - $this->expectException(PHPUnit_Framework_Error_Warning::class); - $this->expectExceptionMessage( - 'Cache-Control header has already been set. ' - . 'Please use HTTPCacheControlMiddleware API to set caching options instead.' - ); - - HTTP::add_cache_headers($response); } public function testConfigVary() { $body = "

Mysite

"; $response = new HTTPResponse($body, 200); - HTTPCacheControlMiddleware::singleton()->setMaxAge(30); - HTTP::add_cache_headers($response); + HTTPCacheControlMiddleware::singleton() + ->setMaxAge(30) + ->setVary('X-Requested-With, X-Forwarded-Protocol'); + $this->addCacheHeaders($response); + // Vary set properly $v = $response->getHeader('Vary'); - $this->assertNotEmpty($v); - $this->assertContains("X-Forwarded-Protocol", $v); $this->assertContains("X-Requested-With", $v); $this->assertNotContains("Cookie", $v); $this->assertNotContains("User-Agent", $v); $this->assertNotContains("Accept", $v); - HTTP::config()->update('vary', ''); + // No vary + HTTPCacheControlMiddleware::singleton() + ->setMaxAge(30) + ->setVary(null); HTTPCacheControlMiddleware::reset(); + HTTPCacheControlMiddleware::config() + ->set('defaultVary', []); $response = new HTTPResponse($body, 200); - HTTP::add_cache_headers($response); - + $this->addCacheHeaders($response); $v = $response->getHeader('Vary'); $this->assertEmpty($v); } @@ -395,4 +394,23 @@ class HTTPTest extends FunctionalTest } ); } + + /** + * Process cache headers on a response + * + * @param HTTPResponse $response + */ + protected function addCacheHeaders(HTTPResponse $response) + { + // Mock request + $session = new Session([]); + $request = new HTTPRequest('GET', '/'); + $request->setSession($session); + + // Run middleware + HTTPCacheControlMiddleware::singleton() + ->process($request, function (HTTPRequest $request) use ($response) { + return $response; + }); + } } From 92746924159e9667429d0fa5ee8c7d5ed34ea514 Mon Sep 17 00:00:00 2001 From: Damian Mooyman Date: Thu, 14 Jun 2018 11:46:47 +1200 Subject: [PATCH 23/34] Fix core tests --- src/Control/Middleware/ChangeDetectionMiddleware.php | 3 +++ src/Control/Middleware/HTTPCacheControlMiddleware.php | 3 +++ 2 files changed, 6 insertions(+) diff --git a/src/Control/Middleware/ChangeDetectionMiddleware.php b/src/Control/Middleware/ChangeDetectionMiddleware.php index 833483134..4980a95b7 100644 --- a/src/Control/Middleware/ChangeDetectionMiddleware.php +++ b/src/Control/Middleware/ChangeDetectionMiddleware.php @@ -25,6 +25,9 @@ class ChangeDetectionMiddleware implements HTTPMiddleware { /** @var HTTPResponse $response */ $response = $delegate($request); + if (!$response) { + return null; + } // Ignore etag for no-store $cacheControl = $response->getHeader('Cache-Control'); diff --git a/src/Control/Middleware/HTTPCacheControlMiddleware.php b/src/Control/Middleware/HTTPCacheControlMiddleware.php index c91d9c641..7aaae088a 100644 --- a/src/Control/Middleware/HTTPCacheControlMiddleware.php +++ b/src/Control/Middleware/HTTPCacheControlMiddleware.php @@ -43,6 +43,9 @@ class HTTPCacheControlMiddleware implements HTTPMiddleware, Resettable } catch (HTTPResponse_Exception $ex) { $response = $ex->getResponse(); } + if (!$response) { + return null; + } // Update state based on current request and response objects $this->augmentState($request, $response); From 163f1523e91e4096918ff3296342adc5d143ce03 Mon Sep 17 00:00:00 2001 From: Damian Mooyman Date: Thu, 14 Jun 2018 11:55:05 +1200 Subject: [PATCH 24/34] Add upgrade rule for HTTPCacheControl --- .upgrade.yml | 1 + src/Control/Middleware/HTTPCacheControlMiddleware.php | 1 - tests/php/Core/Startup/ErrorControlChainMiddlewareTest.php | 1 - 3 files changed, 1 insertion(+), 2 deletions(-) diff --git a/.upgrade.yml b/.upgrade.yml index e4e2c392a..42584cb49 100644 --- a/.upgrade.yml +++ b/.upgrade.yml @@ -222,6 +222,7 @@ mappings: CookieJar: SilverStripe\Control\CookieJar Director: SilverStripe\Control\Director FlushRequestFilter: SilverStripe\Control\Middleware\FlushMiddleware + HTTPCacheControl: SilverStripe\Control\Middleware\HTTPCacheControlMiddleware HTTP: SilverStripe\Control\HTTP SS_HTTPRequest: SilverStripe\Control\HTTPRequest SS_HTTPResponse: SilverStripe\Control\HTTPResponse diff --git a/src/Control/Middleware/HTTPCacheControlMiddleware.php b/src/Control/Middleware/HTTPCacheControlMiddleware.php index 7aaae088a..3d7fdc01d 100644 --- a/src/Control/Middleware/HTTPCacheControlMiddleware.php +++ b/src/Control/Middleware/HTTPCacheControlMiddleware.php @@ -3,7 +3,6 @@ namespace SilverStripe\Control\Middleware; use InvalidArgumentException; -use SilverStripe\Control\HTTP; use SilverStripe\Control\HTTPRequest; use SilverStripe\Control\HTTPResponse; use SilverStripe\Control\HTTPResponse_Exception; diff --git a/tests/php/Core/Startup/ErrorControlChainMiddlewareTest.php b/tests/php/Core/Startup/ErrorControlChainMiddlewareTest.php index d1e4175f8..5cce89d3e 100644 --- a/tests/php/Core/Startup/ErrorControlChainMiddlewareTest.php +++ b/tests/php/Core/Startup/ErrorControlChainMiddlewareTest.php @@ -2,7 +2,6 @@ namespace SilverStripe\Core\Tests\Startup; -use SilverStripe\Control\Cookie; use SilverStripe\Control\HTTPApplication; use SilverStripe\Control\HTTPRequest; use SilverStripe\Control\HTTPResponse; From c271a43904c3cd85d2ca35654f237821b27b1861 Mon Sep 17 00:00:00 2001 From: Damian Mooyman Date: Thu, 14 Jun 2018 12:19:55 +1200 Subject: [PATCH 25/34] Linting fixes --- tests/php/Control/HTTPTest.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/php/Control/HTTPTest.php b/tests/php/Control/HTTPTest.php index 052186ca1..517a57020 100644 --- a/tests/php/Control/HTTPTest.php +++ b/tests/php/Control/HTTPTest.php @@ -410,7 +410,7 @@ class HTTPTest extends FunctionalTest // Run middleware HTTPCacheControlMiddleware::singleton() ->process($request, function (HTTPRequest $request) use ($response) { - return $response; - }); + return $response; + }); } } From 3ce8ab3adc3d5eb3046530973d2c5c6edeebc15c Mon Sep 17 00:00:00 2001 From: Damian Mooyman Date: Thu, 14 Jun 2018 13:01:27 +1200 Subject: [PATCH 26/34] Improve handling of deprecated apis --- docs/en/04_Changelogs/4.2.0.md | 12 +++++ src/Control/HTTP.php | 44 ++++++++++++++----- .../Middleware/HTTPCacheControlMiddleware.php | 4 ++ src/Control/RSS/RSSFeed.php | 5 ++- src/ORM/DataObject.php | 7 --- 5 files changed, 53 insertions(+), 19 deletions(-) diff --git a/docs/en/04_Changelogs/4.2.0.md b/docs/en/04_Changelogs/4.2.0.md index 807985480..d94fbc699 100644 --- a/docs/en/04_Changelogs/4.2.0.md +++ b/docs/en/04_Changelogs/4.2.0.md @@ -236,6 +236,18 @@ that the response should be considered not cacheable. See [Developer Guide: Performance > HTTP Cache Headers](/developer_guide/performance/http_cache_headers) for details on the new API. +#### Disabling legacy cache headers + +In order to forcibly disable all deprecated HTTP APIs you can set the below config: + +```yaml +SilverStripe\Control\HTTP: + ignoreDeprecatedCaching: true +``` + +This will ensure that any code paths that use the old API will not interefere with upgraded code +that interferes with the new behaviour. + #### Example Usage ##### Global opt-in for page content diff --git a/src/Control/HTTP.php b/src/Control/HTTP.php index d50a82091..1e5bcb25a 100644 --- a/src/Control/HTTP.php +++ b/src/Control/HTTP.php @@ -53,6 +53,13 @@ class HTTP */ private static $disable_http_cache = false; + /** + * Set to true to disable all deprecated HTTP Cache settings + * + * @var bool + */ + private static $ignoreDeprecatedCaching = false; + /** * Mapping of extension to mime types * @@ -424,6 +431,11 @@ class HTTP { Deprecation::notice('5.0', 'Headers are added automatically by HTTPCacheControlMiddleware instead.'); + // Skip if deprecated API is disabled + if (Config::inst()->get(HTTP::class, 'ignoreDeprecatedCaching')) { + return; + } + // Ensure a valid response object is provided if (!$response instanceof HTTPResponse) { user_error("HTTP::add_cache_headers() must be passed an HTTPResponse object", E_USER_WARNING); @@ -449,11 +461,30 @@ class HTTP /** @var HTTPRequest $request */ $request = Injector::inst()->get(HTTPRequest::class); - $config = Config::forClass(__CLASS__); + // Run middleware + ChangeDetectionMiddleware::singleton()->process($request, function (HTTPRequest $request) use ($response) { + return HTTPCacheControlMiddleware::singleton()->process($request, function (HTTPRequest $request) use ($response) { + return $response; + }); + }); + } + + /** + * Ensure that all deprecated HTTP cache settings are respected + * + * @deprecated 4.2..5.0 Use HTTPCacheControlMiddleware instead + * @param HTTPRequest $request + * @param HTTPResponse $response + */ + public static function augmentState(HTTPRequest $request, HTTPResponse $response) + { + // Skip if deprecated API is disabled + $config = Config::forClass(HTTP::class); + if ($config->get('ignoreDeprecatedCaching')) { + return; + } - // Get current cache control state $cacheControlMiddleware = HTTPCacheControlMiddleware::singleton(); - $changeDetectionMiddleware = ChangeDetectionMiddleware::singleton(); // if http caching is disabled by config, disable it - used on dev environments due to frequently changing // templates and other data. will be overridden by forced publicCache(true) or privateCache(true) calls @@ -489,13 +520,6 @@ class HTTP Deprecation::notice('5.0', 'Etag should not be set explicitly'); $response->addHeader('ETag', self::$etag); } - - // Run middleware - $changeDetectionMiddleware->process($request, function (HTTPRequest $request) use ($cacheControlMiddleware, $response) { - return $cacheControlMiddleware->process($request, function (HTTPRequest $request) use ($response) { - return $response; - }); - }); } /** diff --git a/src/Control/Middleware/HTTPCacheControlMiddleware.php b/src/Control/Middleware/HTTPCacheControlMiddleware.php index 3d7fdc01d..5038493a7 100644 --- a/src/Control/Middleware/HTTPCacheControlMiddleware.php +++ b/src/Control/Middleware/HTTPCacheControlMiddleware.php @@ -3,6 +3,7 @@ namespace SilverStripe\Control\Middleware; use InvalidArgumentException; +use SilverStripe\Control\HTTP; use SilverStripe\Control\HTTPRequest; use SilverStripe\Control\HTTPResponse; use SilverStripe\Control\HTTPResponse_Exception; @@ -49,6 +50,9 @@ class HTTPCacheControlMiddleware implements HTTPMiddleware, Resettable // Update state based on current request and response objects $this->augmentState($request, $response); + // Update state based on deprecated HTTP settings + HTTP::augmentState($request, $response); + // Add all headers to this response object $this->applyToResponse($response); diff --git a/src/Control/RSS/RSSFeed.php b/src/Control/RSS/RSSFeed.php index 12bc4e876..804c7c756 100644 --- a/src/Control/RSS/RSSFeed.php +++ b/src/Control/RSS/RSSFeed.php @@ -2,6 +2,7 @@ namespace SilverStripe\Control\RSS; +use SilverStripe\Control\Middleware\HTTPCacheControlMiddleware; use SilverStripe\ORM\SS_List; use SilverStripe\ORM\ArrayList; use SilverStripe\ORM\FieldType\DBHTMLText; @@ -226,11 +227,11 @@ class RSSFeed extends ViewableData $response = Controller::curr()->getResponse(); if (is_int($this->lastModified)) { - HTTP::register_modification_timestamp($this->lastModified); + HTTPCacheControlMiddleware::singleton()->registerModificationDate($this->lastModified); $response->addHeader("Last-Modified", gmdate("D, d M Y H:i:s", $this->lastModified) . ' GMT'); } if (!empty($this->etag)) { - HTTP::register_etag($this->etag); + $response->addHeader('ETag', "\"{$this->etag}\""); } $response->addHeader("Content-Type", "application/rss+xml; charset=utf-8"); diff --git a/src/ORM/DataObject.php b/src/ORM/DataObject.php index 04edcb964..d66a25d2b 100644 --- a/src/ORM/DataObject.php +++ b/src/ORM/DataObject.php @@ -380,13 +380,6 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity $this->original = $this->record; - // Keep track of the modification date of all the data sourced to make this page - // From this we create a Last-Modified HTTP header - if (isset($record['LastEdited'])) { - HTTPCacheControlMiddleware::singleton() - ->registerModificationDate($record['LastEdited']); - } - // Must be called after parent constructor if (!$isSingleton && (!isset($this->record['ID']) || !$this->record['ID'])) { $this->populateDefaults(); From 92ab6b63b1596a9b4bb14a4cd60a0c65435363f4 Mon Sep 17 00:00:00 2001 From: Damian Mooyman Date: Thu, 14 Jun 2018 13:54:06 +1200 Subject: [PATCH 27/34] Update recipe branching --- .travis.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.travis.yml b/.travis.yml index 53f0af57b..e94706d0d 100644 --- a/.travis.yml +++ b/.travis.yml @@ -87,8 +87,8 @@ before_script: - mkdir ./public - if [[ $DB == PGSQL ]]; then composer require silverstripe/postgresql:2.0.x-dev --no-update; fi - if [[ $DB == SQLITE ]]; then composer require silverstripe/sqlite3:2.0.x-dev --no-update; fi - - composer require silverstripe/recipe-testing:^1 silverstripe/recipe-core:1.2.x-dev silverstripe/admin:1.2.x-dev silverstripe/versioned:1.2.x-dev --no-update - - if [[ $PHPUNIT_TEST == cms ]] || [[ $BEHAT_TEST == cms ]]; then composer require silverstripe/recipe-cms:1.2.x-dev --no-update; fi + - composer require silverstripe/recipe-testing:^1 silverstripe/recipe-core:4.2.x-dev silverstripe/admin:1.2.x-dev silverstripe/versioned:1.2.x-dev --no-update + - if [[ $PHPUNIT_TEST == cms ]] || [[ $BEHAT_TEST == cms ]]; then composer require silverstripe/recipe-cms:4.2.x-dev --no-update; fi - if [[ $PHPCS_TEST ]]; then composer global require squizlabs/php_codesniffer:^3 --prefer-dist --no-interaction --no-progress --no-suggest -o; fi - composer install --prefer-source --no-interaction --no-progress --no-suggest --optimize-autoloader --verbose --profile From 6b8f63c4d525d6407eb4fef8c454cc96a3970f96 Mon Sep 17 00:00:00 2001 From: Damian Mooyman Date: Thu, 14 Jun 2018 14:11:31 +1200 Subject: [PATCH 28/34] Refactor redirect cache busting into middleware --- docs/en/04_Changelogs/4.2.0.md | 4 ++++ src/Control/Middleware/HTTPCacheControlMiddleware.php | 5 +++++ src/Control/RequestHandler.php | 3 --- src/Forms/Form.php | 6 +----- 4 files changed, 10 insertions(+), 8 deletions(-) diff --git a/docs/en/04_Changelogs/4.2.0.md b/docs/en/04_Changelogs/4.2.0.md index d94fbc699..f942dea09 100644 --- a/docs/en/04_Changelogs/4.2.0.md +++ b/docs/en/04_Changelogs/4.2.0.md @@ -357,3 +357,7 @@ class PageController extends ContentController Note this is different from `Vary: Accept-Encoding`, which is important for compression (e.g. gzip), and usually added by other layers such as Apache's mod_gzip. + * DataObject no longer automatically invokes `HTTP::register_modification_date` + with LastModified on construction. User code should instead invoke + `HTTPCacheControlMiddleware::singleton()->registerModificationDate()` explicitly. + diff --git a/src/Control/Middleware/HTTPCacheControlMiddleware.php b/src/Control/Middleware/HTTPCacheControlMiddleware.php index 5038493a7..ea331a48a 100644 --- a/src/Control/Middleware/HTTPCacheControlMiddleware.php +++ b/src/Control/Middleware/HTTPCacheControlMiddleware.php @@ -769,5 +769,10 @@ class HTTPCacheControlMiddleware implements HTTPMiddleware, Resettable // Even if publicCache(true) is specified, errors will be uncacheable $this->disableCache(true); } + + // Don't cache redirects + if ($response->isRedirect()) { + $this->disableCache(true); + } } } diff --git a/src/Control/RequestHandler.php b/src/Control/RequestHandler.php index 5ab831c2f..369799f4c 100644 --- a/src/Control/RequestHandler.php +++ b/src/Control/RequestHandler.php @@ -657,9 +657,6 @@ class RequestHandler extends ViewableData */ public function redirectBack() { - // Don't cache the redirect back ever - HTTPCacheControlMiddleware::singleton()->disableCache(true); - // Prefer to redirect to ?BackURL, but fall back to Referer header // As a last resort redirect to base url $url = $this->getBackURL() diff --git a/src/Forms/Form.php b/src/Forms/Form.php index 186ebf659..0619f78f5 100644 --- a/src/Forms/Form.php +++ b/src/Forms/Form.php @@ -1812,13 +1812,9 @@ class Form extends ViewableData implements HasRequestHandler /** * Can the body of this form be cached? * - * Figure out if we can cache this form - * - forms with validation shouldn't be cached, because their error messages won't be shown - * - forms with security tokens shouldn't be cached because security tokens expire - * * @return bool */ - public function canBeCached() + protected function canBeCached() { if ($this->getSecurityToken()->isEnabled()) { return false; From 4c30c9ac2f549b1f4e294848d62d6a5dd2515cff Mon Sep 17 00:00:00 2001 From: Ingo Schommer Date: Thu, 14 Jun 2018 10:56:05 +1200 Subject: [PATCH 29/34] Clarified cases in which forms prevent caching --- .../02_Developer_Guides/03_Forms/04_Form_Security.md | 10 ++++++---- .../08_Performance/02_HTTP_Cache_Headers.md | 3 ++- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/docs/en/02_Developer_Guides/03_Forms/04_Form_Security.md b/docs/en/02_Developer_Guides/03_Forms/04_Form_Security.md index 82d270c9d..a6e21cfe2 100644 --- a/docs/en/02_Developer_Guides/03_Forms/04_Form_Security.md +++ b/docs/en/02_Developer_Guides/03_Forms/04_Form_Security.md @@ -78,10 +78,12 @@ module if required. The module provides an consistent API for allowing third-par ## Data disclosure through HTTP Caching (since 4.2.0) -Forms, and particularly their responses, can contain sensitive data (such as when data is pre-populated or re-posted due -to validation errors). This data can inadvertently be stored either in a user's browser cache or in an intermediary -cache such as a CDN or other caching-proxy. If incorrect `Cache-Conrol` headers are used, private data may be cached and -accessible publicly through the CDN. +Forms, and particularly their responses, can contain sensitive or user-specific data. +Forms can prepopulate submissions when a form is redisplayed with validation errors, +and they by default contain CSRF tokens unique to the user's session. +This data can inadvertently be stored either in a user's browser cache or in an intermediary +cache such as a CDN or other caching-proxy. If incorrect `Cache-Control` headers are used, private data may be cached and +accessible publicly through the CDN. To ensure this doesn't happen SilverStripe adds `Cache-Control: no-store, no-cache, must-revalidate` headers to any forms that have validators or security tokens (all of them by default) applied to them; this ensures that CDNs diff --git a/docs/en/02_Developer_Guides/08_Performance/02_HTTP_Cache_Headers.md b/docs/en/02_Developer_Guides/08_Performance/02_HTTP_Cache_Headers.md index 232bc350f..227696ae0 100644 --- a/docs/en/02_Developer_Guides/08_Performance/02_HTTP_Cache_Headers.md +++ b/docs/en/02_Developer_Guides/08_Performance/02_HTTP_Cache_Headers.md @@ -162,7 +162,8 @@ Use case: By default, forms include a [CSRF token](/developer_guides/forms/form_ which starts a session with a value that's unique to the visitor, which makes the output uncacheable. But any subsequent requests by this visitor will also carry a session, leading to uncacheable output for this visitor. This is the case even if the output does not contain any forms, -and does not vary for this particular visitor. +and does not vary for this particular visitor. Forms can also contain submission data +when they're redisplayed after a validation error. ```php Date: Thu, 14 Jun 2018 13:31:25 +1200 Subject: [PATCH 30/34] List deprecated APIs in changelog --- .../08_Performance/02_HTTP_Cache_Headers.md | 4 ++-- docs/en/04_Changelogs/4.2.0.md | 17 ++++++++++++----- 2 files changed, 14 insertions(+), 7 deletions(-) diff --git a/docs/en/02_Developer_Guides/08_Performance/02_HTTP_Cache_Headers.md b/docs/en/02_Developer_Guides/08_Performance/02_HTTP_Cache_Headers.md index 227696ae0..d1274d7c6 100644 --- a/docs/en/02_Developer_Guides/08_Performance/02_HTTP_Cache_Headers.md +++ b/docs/en/02_Developer_Guides/08_Performance/02_HTTP_Cache_Headers.md @@ -21,8 +21,8 @@ are a great way to learn about HTTP caching. In order to support developers in making safe choices around HTTP caching, we're using a `HTTPCacheControlMiddleware` class to control if a response -should be considered public or private. This is an abstraction on existing -lowlevel APIs like `HTTP::add_cache_headers()` and `HTTPResponse->addHeader()`. +should be considered public or private. This is an abstraction on the +`HTTPResponse->addHeader()` lowlevel API. The `HTTPCacheControlMiddleware` API makes it easier to express your caching preferences without running the risk of overriding essential core safety measures. diff --git a/docs/en/04_Changelogs/4.2.0.md b/docs/en/04_Changelogs/4.2.0.md index f942dea09..b63fea5b1 100644 --- a/docs/en/04_Changelogs/4.2.0.md +++ b/docs/en/04_Changelogs/4.2.0.md @@ -207,9 +207,9 @@ SilverStripe\Core\Injector\Injector: #### Overview In order to support developers in making safe choices around HTTP caching, -we've introduced a `HTTPCacheControlMiddleware` class to control if a response -should be considered public or private. This is an abstraction on existing -lowlevel APIs like `HTTP::add_cache_headers()` and `HTTPResponse->addHeader()`. +we're using a `HTTPCacheControlMiddleware` class to control if a response +should be considered public or private. This is an abstraction on the +`HTTPResponse->addHeader()` lowlevel API. This change introduces smaller but necessary changes to HTTP caching headers sent by SilverStripe. If you are relying on HTTP caching in your implementation, @@ -344,7 +344,7 @@ class PageController extends ContentController } ``` -#### Detailed Cache-Control Changes: +#### Detailed Cache-Control Changes * Added `Cache-Control: no-store` header to default responses, to prevent intermediary HTTP proxies (e.g. CDNs) from caching unless developers opt-in @@ -360,4 +360,11 @@ class PageController extends ContentController * DataObject no longer automatically invokes `HTTP::register_modification_date` with LastModified on construction. User code should instead invoke `HTTPCacheControlMiddleware::singleton()->registerModificationDate()` explicitly. - + * Deprecated `HTTP::add_cache_headers()`. Headers are added automatically by `HTTPCacheControlMiddleware` instead. + * Deprecated `HTTP::set_cache_age()`. Use `HTTPCacheControlMiddleware::singleton()->setMaxAge($age)` + * Deprecated `HTTP.cache_ajax_requests`. Use `HTTPCacheControlMiddleware::disableCache()` instead + * Deprecated `HTTP.modification_date`. Handled by `HTTPCacheControlMiddleware` + * Deprecated `HTTP.disable_http_cache`. Use `HTTPCacheControlMiddleware.defaultState` and `defaultForcingLevel` instead + * Deprecated `HTTP::register_modification_date()`. Use `HTTPCacheControlMiddleware::registerModificationDate()` instead + * Deprecated `HTTP::register_modification_timestamp()`. Use `HTTPCacheControlMiddleware::registerModificationDate()` instead + * Deprecated `HTTP::register_etag()`. Use `HTTPCacheControlMiddleware::ETagMiddleware()` instead \ No newline at end of file From 513e0891d365a1d92fd65c7fd4f77255908b6baf Mon Sep 17 00:00:00 2001 From: Ingo Schommer Date: Thu, 14 Jun 2018 13:31:34 +1200 Subject: [PATCH 31/34] Clarify function of registerModificationDate() --- src/Control/Middleware/HTTPCacheControlMiddleware.php | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/Control/Middleware/HTTPCacheControlMiddleware.php b/src/Control/Middleware/HTTPCacheControlMiddleware.php index ea331a48a..048303eb1 100644 --- a/src/Control/Middleware/HTTPCacheControlMiddleware.php +++ b/src/Control/Middleware/HTTPCacheControlMiddleware.php @@ -109,8 +109,8 @@ class HTTPCacheControlMiddleware implements HTTPMiddleware, Resettable /** * Forcing level of previous setting; higher number wins - * Combination of consts belo - *w + * Combination of consts below + * * @var int */ protected $forcingLevel = null; @@ -257,7 +257,8 @@ class HTTPCacheControlMiddleware implements HTTPMiddleware, Resettable /** - * Register a modification date. Used to calculate the Modification-Date http header + * Register a modification date. Used to calculate the "Last-Modified" HTTP header. + * Can be called multiple times, and will automatically retain the most recent date. * * @param string|int $date Date string or timestamp * @return HTTPCacheControlMiddleware From 24e9253b8775783d003769f63e34b893278a42e1 Mon Sep 17 00:00:00 2001 From: Ingo Schommer Date: Thu, 14 Jun 2018 13:35:40 +1200 Subject: [PATCH 32/34] Mention Last-Modified change --- docs/en/04_Changelogs/4.2.0.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/docs/en/04_Changelogs/4.2.0.md b/docs/en/04_Changelogs/4.2.0.md index b63fea5b1..1b6e5e6df 100644 --- a/docs/en/04_Changelogs/4.2.0.md +++ b/docs/en/04_Changelogs/4.2.0.md @@ -357,6 +357,9 @@ class PageController extends ContentController Note this is different from `Vary: Accept-Encoding`, which is important for compression (e.g. gzip), and usually added by other layers such as Apache's mod_gzip. + * No longer sets `Last-Modified` date in HTTP response headers in `DataObject::__construct()`. + Uses `ETag` calculation based on response body which is more accurate, + and resilient against partial and object caching which can produce stale `Last-Modified` values. * DataObject no longer automatically invokes `HTTP::register_modification_date` with LastModified on construction. User code should instead invoke `HTTPCacheControlMiddleware::singleton()->registerModificationDate()` explicitly. From 779865e29fc7ea6f1a8784de1ba8bd315434bc9f Mon Sep 17 00:00:00 2001 From: Ingo Schommer Date: Thu, 14 Jun 2018 14:15:55 +1200 Subject: [PATCH 33/34] merge changelog --- docs/en/04_Changelogs/4.2.0.md | 3 --- 1 file changed, 3 deletions(-) diff --git a/docs/en/04_Changelogs/4.2.0.md b/docs/en/04_Changelogs/4.2.0.md index 1b6e5e6df..8543e735e 100644 --- a/docs/en/04_Changelogs/4.2.0.md +++ b/docs/en/04_Changelogs/4.2.0.md @@ -360,9 +360,6 @@ class PageController extends ContentController * No longer sets `Last-Modified` date in HTTP response headers in `DataObject::__construct()`. Uses `ETag` calculation based on response body which is more accurate, and resilient against partial and object caching which can produce stale `Last-Modified` values. - * DataObject no longer automatically invokes `HTTP::register_modification_date` - with LastModified on construction. User code should instead invoke - `HTTPCacheControlMiddleware::singleton()->registerModificationDate()` explicitly. * Deprecated `HTTP::add_cache_headers()`. Headers are added automatically by `HTTPCacheControlMiddleware` instead. * Deprecated `HTTP::set_cache_age()`. Use `HTTPCacheControlMiddleware::singleton()->setMaxAge($age)` * Deprecated `HTTP.cache_ajax_requests`. Use `HTTPCacheControlMiddleware::disableCache()` instead From b686b86c343e68ebd7d5c3b69b6687f5a88809a0 Mon Sep 17 00:00:00 2001 From: Damian Mooyman Date: Thu, 14 Jun 2018 15:54:31 +1200 Subject: [PATCH 34/34] Session now prevents cache headers being sent unintentionally --- src/Control/Session.php | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/src/Control/Session.php b/src/Control/Session.php index 9cba852b9..d0e68f35e 100644 --- a/src/Control/Session.php +++ b/src/Control/Session.php @@ -128,6 +128,15 @@ class Session */ private static $cookie_secure = false; + /** + * Name of session cache limiter to use. + * Defaults to '' to disable cache limiter entirely. + * + * @see https://secure.php.net/manual/en/function.session-cache-limiter.php + * @var string|null + */ + private static $sessionCacheLimiter = ''; + /** * Session data. * Will be null if session has not been started @@ -275,6 +284,11 @@ class Session session_name('SECSESSID'); } + $limiter = $this->config()->get('sessionCacheLimiter'); + if (isset($limiter)) { + session_cache_limiter($limiter); + } + session_start(); $this->data = isset($_SESSION) ? $_SESSION : array();