diff --git a/control/HTTPResponse.php b/control/HTTPResponse.php index 5489a8949..9e7c12190 100644 --- a/control/HTTPResponse.php +++ b/control/HTTPResponse.php @@ -234,12 +234,14 @@ class SS_HTTPResponse { } if(in_array($this->statusCode, self::$redirect_codes) && headers_sent($file, $line)) { - $url = $this->headers['Location']; + $url = (string)$this->headers['Location']; + $urlATT = Convert::raw2htmlatt($url); + $urlJS = Convert::raw2js($url); echo - "

Redirecting to " - . "$url... (output started on $file, line $line)

- - "; + "

Redirecting to " + . "$urlATT... (output started on $file, line $line)

+ + "; } else { $line = $file = null; if(!headers_sent($file, $line)) { diff --git a/docs/en/changelogs/3.0.10.md b/docs/en/changelogs/3.0.10.md new file mode 100644 index 000000000..427b54498 --- /dev/null +++ b/docs/en/changelogs/3.0.10.md @@ -0,0 +1,12 @@ +# 3.0.10 + +## Overview + + * Security: Partially cached content from stage or other reading modes is no longer emitted to live + +## Upgrading + + * If relying on partial caching of content between logged in users, be aware that the cache is now automatically + segmented based on both the current member ID, and the versioned reading mode. If this is not an appropriate + method (such as if the same content is served to logged in users within partial caching) then it is necessary + to adjust the config value of `SSViewer::global_key` to something more or less sensitive. \ No newline at end of file diff --git a/docs/en/reference/form-field-types.md b/docs/en/reference/form-field-types.md index 3d45812fc..5aeef002f 100644 --- a/docs/en/reference/form-field-types.md +++ b/docs/en/reference/form-field-types.md @@ -28,7 +28,7 @@ This is a highlevel overview of available `[api:FormField]` subclasses. An autom * `[api:DatetimeField]`: Combined date- and time field. * `[api:EmailField]`: Text input field with validation for correct email format according to RFC 2822. * `[api:GroupedDropdownField]`: Grouped dropdown, using tags. - * `[api:HTMLEditorField]. + * `[api:HtmlEditorField]`. * `[api:MoneyField]`: A form field that can save into a `[api:Money]` database field. * `[api:NumericField]`: Text input field with validation for numeric values. * `[api:OptionsetField]`: Set of radio buttons designed to emulate a dropdown. diff --git a/docs/en/reference/partial-caching.md b/docs/en/reference/partial-caching.md index 5bf957596..64e5c8366 100644 --- a/docs/en/reference/partial-caching.md +++ b/docs/en/reference/partial-caching.md @@ -51,6 +51,19 @@ From a block that shows a summary of the page edits if administrator, nothing if <% cached 'loginblock', LastEdited, CurrentMember.isAdmin %> +An additional global key is incorporated in the cache lookup. The default value for this is +`$CurrentReadingMode, $CurrentUser.ID`, which ensures that the current `[api:Versioned]` state and user ID are +used. This may be configured by changing the config value of `SSViewer.global_key`. It is also necessary +to flush the template caching when modifying this config, as this key is cached within the template itself. + +For example, to ensure that the cache is configured to respect another variable, and if the current logged in +user does not influence your template content, you can update this key as below; + + :::yaml + SSViewer: + global_key: '$CurrentReadingMode, $Locale' + + ## Aggregates Often you want to invalidate a cache when any in a set of objects change, or when the objects in a relationship change. diff --git a/model/Versioned.php b/model/Versioned.php index 38844e9c5..2f3da904c 100644 --- a/model/Versioned.php +++ b/model/Versioned.php @@ -8,8 +8,7 @@ * @package framework * @subpackage model */ -class Versioned extends DataExtension { - +class Versioned extends DataExtension implements TemplateGlobalProvider { /** * An array of possible stages. * @var array @@ -1339,6 +1338,12 @@ class Versioned extends DataExtension { public function getDefaultStage() { return $this->defaultStage; } + + public static function get_template_global_variables() { + return array( + 'CurrentReadingMode' => 'get_reading_mode' + ); + } } /** diff --git a/tests/view/SSViewerCacheBlockTest.php b/tests/view/SSViewerCacheBlockTest.php index 0f81a099e..ceb1e4be4 100644 --- a/tests/view/SSViewerCacheBlockTest.php +++ b/tests/view/SSViewerCacheBlockTest.php @@ -20,9 +20,29 @@ class SSViewerCacheBlockTest_Model extends DataObject implements TestOnly { } } +class SSViewerCacheBlockTest_VersionedModel extends DataObject implements TestOnly { + + protected $entropy = 'default'; + + public static $extensions = array( + "Versioned('Stage', 'Live')" + ); + + public function setEntropy($entropy) { + $this->entropy = $entropy; + } + + public function Inspect() { + return $this->entropy . ' ' . Versioned::get_reading_mode(); + } +} + class SSViewerCacheBlockTest extends SapphireTest { - protected $extraDataObjects = array('SSViewerCacheBlockTest_Model'); + protected $extraDataObjects = array( + 'SSViewerCacheBlockTest_Model', + 'SSViewerCacheBlockTest_VersionedModel' + ); protected $data = null; @@ -37,8 +57,7 @@ class SSViewerCacheBlockTest extends SapphireTest { if ($data === null) $data = $this->data; if (is_array($data)) $data = $this->data->customise($data); - $viewer = SSViewer::fromString($template); - return $viewer->process($data); + return SSViewer::execute_string($template, $data); } public function testParsing() { @@ -104,6 +123,77 @@ class SSViewerCacheBlockTest extends SapphireTest { $this->assertEquals($this->_runtemplate('<% cached %>$Foo<% end_cached %>', array('Foo' => 1)), '1'); $this->assertEquals($this->_runtemplate('<% cached %>$Foo<% end_cached %>', array('Foo' => 2)), '1'); } + + public function testVersionedCache() { + + $origStage = Versioned::current_stage(); + + // Run without caching in stage to prove data is uncached + $this->_reset(false); + Versioned::reading_stage("Stage"); + $data = new SSViewerCacheBlockTest_VersionedModel(); + $data->setEntropy('default'); + $this->assertEquals( + 'default Stage.Stage', + SSViewer::execute_string('<% cached %>$Inspect<% end_cached %>', $data) + ); + $data = new SSViewerCacheBlockTest_VersionedModel(); + $data->setEntropy('first'); + $this->assertEquals( + 'first Stage.Stage', + SSViewer::execute_string('<% cached %>$Inspect<% end_cached %>', $data) + ); + + // Run without caching in live to prove data is uncached + $this->_reset(false); + Versioned::reading_stage("Live"); + $data = new SSViewerCacheBlockTest_VersionedModel(); + $data->setEntropy('default'); + $this->assertEquals( + 'default Stage.Live', + $this->_runtemplate('<% cached %>$Inspect<% end_cached %>', $data) + ); + $data = new SSViewerCacheBlockTest_VersionedModel(); + $data->setEntropy('first'); + $this->assertEquals( + 'first Stage.Live', + $this->_runtemplate('<% cached %>$Inspect<% end_cached %>', $data) + ); + + // Then with caching, initially in draft, and then in live, to prove that + // changing the versioned reading mode doesn't cache between modes, but it does + // within them + $this->_reset(true); + Versioned::reading_stage("Stage"); + $data = new SSViewerCacheBlockTest_VersionedModel(); + $data->setEntropy('default'); + $this->assertEquals( + 'default Stage.Stage', + $this->_runtemplate('<% cached %>$Inspect<% end_cached %>', $data) + ); + $data = new SSViewerCacheBlockTest_VersionedModel(); + $data->setEntropy('first'); + $this->assertEquals( + 'default Stage.Stage', // entropy should be ignored due to caching + $this->_runtemplate('<% cached %>$Inspect<% end_cached %>', $data) + ); + + Versioned::reading_stage('Live'); + $data = new SSViewerCacheBlockTest_VersionedModel(); + $data->setEntropy('first'); + $this->assertEquals( + 'first Stage.Live', // First hit in live, so display current entropy + $this->_runtemplate('<% cached %>$Inspect<% end_cached %>', $data) + ); + $data = new SSViewerCacheBlockTest_VersionedModel(); + $data->setEntropy('second'); + $this->assertEquals( + 'first Stage.Live', // entropy should be ignored due to caching + $this->_runtemplate('<% cached %>$Inspect<% end_cached %>', $data) + ); + + Versioned::reading_stage($origStage); + } /** * Test that cacheblocks conditionally cache with if diff --git a/tests/view/SSViewerTest.php b/tests/view/SSViewerTest.php index 01ff00524..474ea1cf6 100644 --- a/tests/view/SSViewerTest.php +++ b/tests/view/SSViewerTest.php @@ -85,9 +85,8 @@ class SSViewerTest extends SapphireTest { * Small helper to render templates from strings */ public function render($templateString, $data = null) { - $t = SSViewer::fromString($templateString); if(!$data) $data = new SSViewerTestFixture(); - return $t->process($data); + return SSViewer::execute_string($templateString, $data); } public function testRequirements() { diff --git a/view/SSTemplateParser.php b/view/SSTemplateParser.php index f0f6f4f33..6a625140a 100644 --- a/view/SSTemplateParser.php +++ b/view/SSTemplateParser.php @@ -67,7 +67,7 @@ class SSTemplateParseException extends Exception { * * Angle Bracket: angle brackets "<" and ">" are used to eat whitespace between template elements * N: eats white space including newlines (using in legacy _t support) - * + * * @package framework * @subpackage view */ @@ -2938,10 +2938,25 @@ class SSTemplateParser extends Parser implements TemplateParser { function CacheBlock_CacheBlockTemplate(&$res, $sub){ // Get the block counter $block = ++$res['subblocks']; - // Build the key for this block from the passed cache key, the block index, and the sha hash of the template - // itself - $key = "'" . sha1($sub['php']) . (isset($res['key']) && $res['key'] ? "_'.sha1(".$res['key'].")" : "'") . - ".'_$block'"; + // Build the key for this block from the global key (evaluated in a closure within the template), + // the passed cache key, the block index, and the sha hash of the template. + $res['php'] .= '$keyExpression = function() use ($scope, $cache) {' . PHP_EOL; + $res['php'] .= '$val = \'\';' . PHP_EOL; + if($globalKey = Config::inst()->get('SSViewer', 'global_key')) { + // Embed the code necessary to evaluate the globalKey directly into the template, + // so that SSTemplateParser only needs to be called during template regeneration. + // Warning: If the global key is changed, it's necessary to flush the template cache. + $parser = new SSTemplateParser($globalKey); + $result = $parser->match_Template(); + if(!$result) throw new SSTemplateParseException('Unexpected problem parsing template', $parser); + $res['php'] .= $result['php'] . PHP_EOL; + } + $res['php'] .= 'return $val;' . PHP_EOL; + $res['php'] .= '};' . PHP_EOL; + $key = 'sha1($keyExpression())' // Global key + . '.\'_' . sha1($sub['php']) // sha of template + . (isset($res['key']) && $res['key'] ? "_'.sha1(".$res['key'].")" : "'") // Passed key + . ".'_$block'"; // block index // Get any condition $condition = isset($res['condition']) ? $res['condition'] : ''; diff --git a/view/SSTemplateParser.php.inc b/view/SSTemplateParser.php.inc index 5a750c7dd..eb467fb14 100644 --- a/view/SSTemplateParser.php.inc +++ b/view/SSTemplateParser.php.inc @@ -673,10 +673,25 @@ class SSTemplateParser extends Parser implements TemplateParser { function CacheBlock_CacheBlockTemplate(&$res, $sub){ // Get the block counter $block = ++$res['subblocks']; - // Build the key for this block from the passed cache key, the block index, and the sha hash of the template - // itself - $key = "'" . sha1($sub['php']) . (isset($res['key']) && $res['key'] ? "_'.sha1(".$res['key'].")" : "'") . - ".'_$block'"; + // Build the key for this block from the global key (evaluated in a closure within the template), + // the passed cache key, the block index, and the sha hash of the template. + $res['php'] .= '$keyExpression = function() use ($scope, $cache) {' . PHP_EOL; + $res['php'] .= '$val = \'\';' . PHP_EOL; + if($globalKey = Config::inst()->get('SSViewer', 'global_key')) { + // Embed the code necessary to evaluate the globalKey directly into the template, + // so that SSTemplateParser only needs to be called during template regeneration. + // Warning: If the global key is changed, it's necessary to flush the template cache. + $parser = new SSTemplateParser($globalKey); + $result = $parser->match_Template(); + if(!$result) throw new SSTemplateParseException('Unexpected problem parsing template', $parser); + $res['php'] .= $result['php'] . PHP_EOL; + } + $res['php'] .= 'return $val;' . PHP_EOL; + $res['php'] .= '};' . PHP_EOL; + $key = 'sha1($keyExpression())' // Global key + . '.\'_' . sha1($sub['php']) // sha of template + . (isset($res['key']) && $res['key'] ? "_'.sha1(".$res['key'].")" : "'") // Passed key + . ".'_$block'"; // block index // Get any condition $condition = isset($res['condition']) ? $res['condition'] : ''; diff --git a/view/SSViewer.php b/view/SSViewer.php index 71271cbed..b5f228280 100644 --- a/view/SSViewer.php +++ b/view/SSViewer.php @@ -48,7 +48,6 @@ class SSViewer_Scope { private $localIndex; - public function __construct($item, $inheritedScope = null) { $this->item = $item; $this->localIndex = 0; @@ -631,6 +630,14 @@ class SSViewer { */ protected $parser; + /* + * Default prepended cache key for partial caching + * + * @var string + * @config + */ + private static $global_key = '$CurrentReadingMode, $CurrentUser.ID'; + /** * Create a template from a string instead of a .ss file * @@ -1083,6 +1090,11 @@ class SSViewer { /** * Execute the given template, passing it the given data. * Used by the <% include %> template tag to process templates. + * + * @param string $template Template name + * @param mixed $data Data context + * @param array $arguments Additional arguments + * @return string Evaluated result */ public static function execute_template($template, $data, $arguments = null, $scope = null) { $v = new SSViewer($template); @@ -1090,6 +1102,23 @@ class SSViewer { return $v->process($data, $arguments, $scope); } + + /** + * Execute the evaluated string, passing it the given data. + * Used by partial caching to evaluate custom cache keys expressed using + * template expressions + * + * @param string $content Input string + * @param mixed $data Data context + * @param array $arguments Additional arguments + * @return string Evaluated result + */ + public static function execute_string($content, $data, $arguments = null) { + $v = SSViewer::fromString($content); + $v->includeRequirements(false); + + return $v->process($data, $arguments); + } public function parseTemplateContent($content, $template="") { return $this->parser->compileString(