From cb2dcc75f1b36e20cb16529d239c7e57c97190c3 Mon Sep 17 00:00:00 2001 From: Robbie Averill Date: Mon, 9 Jan 2017 16:13:39 +1300 Subject: [PATCH 1/6] Add X-Robots-Tag noindex,nofollow header from Security controller to prevent indexing --- security/Security.php | 11 +++++++++++ tests/security/SecurityTest.php | 7 +++++++ 2 files changed, 18 insertions(+) diff --git a/security/Security.php b/security/Security.php index 6e960ccbf..840af3f9d 100644 --- a/security/Security.php +++ b/security/Security.php @@ -146,6 +146,14 @@ class Security extends Controller implements TemplateGlobalProvider { */ private static $frame_options = 'SAMEORIGIN'; + /** + * Value of the X-Robots-Tag header (for the Security section) + * + * @config + * @var string + */ + private static $robots_tag = 'noindex, nofollow'; + /** * Get location of word list file * @@ -326,6 +334,9 @@ class Security extends Controller implements TemplateGlobalProvider { // Prevent clickjacking, see https://developer.mozilla.org/en-US/docs/HTTP/X-Frame-Options $this->getResponse()->addHeader('X-Frame-Options', $this->config()->frame_options); + + // Prevent search engines from indexing the login page + $this->getResponse()->addHeader('X-Robots-Tag', $this->config()->robots_tag); } public function index() { diff --git a/tests/security/SecurityTest.php b/tests/security/SecurityTest.php index 45463d6fe..bd2c8ed0f 100644 --- a/tests/security/SecurityTest.php +++ b/tests/security/SecurityTest.php @@ -574,6 +574,13 @@ class SecurityTest extends FunctionalTest { Security::$force_database_is_ready = $old; } + public function testSecurityControllerSendsRobotsTagHeader() { + $response = $this->get(Config::inst()->get('Security', 'login_url')); + $robotsHeader = $response->getHeader('X-Robots-Tag'); + $this->assertNotNull($robotsHeader); + $this->assertContains('noindex', $robotsHeader); + } + /** * Execute a log-in form using Director::test(). * Helper method for the tests above From 2f6f5b5eff11736e89960fc770f3ef93fe5ef2f7 Mon Sep 17 00:00:00 2001 From: Robbie Averill Date: Wed, 11 Jan 2017 08:26:04 +1300 Subject: [PATCH 2/6] Do not send the header if it is not defined --- security/Security.php | 4 +++- tests/security/SecurityTest.php | 7 +++++++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/security/Security.php b/security/Security.php index 840af3f9d..c6c6cd402 100644 --- a/security/Security.php +++ b/security/Security.php @@ -336,7 +336,9 @@ class Security extends Controller implements TemplateGlobalProvider { $this->getResponse()->addHeader('X-Frame-Options', $this->config()->frame_options); // Prevent search engines from indexing the login page - $this->getResponse()->addHeader('X-Robots-Tag', $this->config()->robots_tag); + if ($this->config()->robots_tag) { + $this->getResponse()->addHeader('X-Robots-Tag', $this->config()->robots_tag); + } } public function index() { diff --git a/tests/security/SecurityTest.php b/tests/security/SecurityTest.php index bd2c8ed0f..a2206e37d 100644 --- a/tests/security/SecurityTest.php +++ b/tests/security/SecurityTest.php @@ -581,6 +581,13 @@ class SecurityTest extends FunctionalTest { $this->assertContains('noindex', $robotsHeader); } + public function testDoNotSendEmptyRobotsHeaderIfNotDefined() { + Config::inst()->update('Security', 'robots_tag', null); + $response = $this->get(Config::inst()->get('Security', 'login_url')); + $robotsHeader = $response->getHeader('X-Robots-Tag'); + $this->assertNull($robotsHeader); + } + /** * Execute a log-in form using Director::test(). * Helper method for the tests above From c7586c68b728dcef51a56d25578d5c0b2eeee0c7 Mon Sep 17 00:00:00 2001 From: Jake Bentvelzen Date: Fri, 13 Jan 2017 15:03:47 +1100 Subject: [PATCH 3/6] feat(ListDecorator): Add setList() function. Useful for keeping a decorator/paginated list configuration intact while modifying the underlying list. --- model/ListDecorator.php | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/model/ListDecorator.php b/model/ListDecorator.php index fc3be6b0c..522efc928 100644 --- a/model/ListDecorator.php +++ b/model/ListDecorator.php @@ -15,8 +15,7 @@ abstract class SS_ListDecorator extends ViewableData implements SS_List, SS_Sort protected $list; public function __construct(SS_List $list) { - $this->list = $list; - $this->failover = $this->list; + $this->setList($list); parent::__construct(); } @@ -30,6 +29,21 @@ abstract class SS_ListDecorator extends ViewableData implements SS_List, SS_Sort return $this->list; } + /** + * Set the list this decorator wraps around. + * + * Useful for keeping a decorator/paginated list configuration intact while modifying + * the underlying list. + * + * @return SS_List + */ + public function setList(SS_List $list) + { + $this->list = $list; + $this->failover = $this->list; + return $this; + } + // PROXIED METHODS --------------------------------------------------------- public function offsetExists($key) { From 1f1fffe73454930c1aef394e9b106a484e6d59ee Mon Sep 17 00:00:00 2001 From: Stephan Bauer Date: Sat, 14 Jan 2017 23:13:47 +0100 Subject: [PATCH 4/6] BUG Ensure correct regeneration of ConfigManifest if only one of the cache files is missing (fixes #6467) --- core/manifest/ConfigManifest.php | 2 +- tests/core/manifest/ConfigManifestTest.php | 77 ++++++++++++++++++++++ 2 files changed, 78 insertions(+), 1 deletion(-) diff --git a/core/manifest/ConfigManifest.php b/core/manifest/ConfigManifest.php index af04ae9e9..e9091bf6f 100644 --- a/core/manifest/ConfigManifest.php +++ b/core/manifest/ConfigManifest.php @@ -99,7 +99,7 @@ class SS_ConfigManifest { } // If we don't have a variantKeySpec (because we're forcing regen, or it just wasn't in the cache), generate it - if (false === $this->variantKeySpec) { + if (false === $this->phpConfigSources || false === $this->variantKeySpec) { $this->regenerate($includeTests); } diff --git a/tests/core/manifest/ConfigManifestTest.php b/tests/core/manifest/ConfigManifestTest.php index a1fac8424..5b14df254 100644 --- a/tests/core/manifest/ConfigManifestTest.php +++ b/tests/core/manifest/ConfigManifestTest.php @@ -152,6 +152,83 @@ class ConfigManifestTest extends SapphireTest { $manifest->__construct(dirname(__FILE__).'/fixtures/configmanifest', false, false); } + /** + * Test cache regeneration if all or some of the cache files are missing + * + * 1. Test regeneration if all cache files are missing + * 2. Test regeneration if 'variant_key_spec' cache file is missing + * 3. Test regeneration if 'php_config_sources' cache file is missing + */ + public function testAutomaticCacheRegeneration(){ + $base = dirname(__FILE__) . '/fixtures/configmanifest'; + + // Test regeneration if all cache files are missing + $manifest = $this->getManifestMock(array('getCache', 'regenerate', 'buildYamlConfigVariant')); + + $manifest->expects($this->once())// regenerate should be called once + ->method('regenerate') + ->with($this->equalTo(false)); // includeTests = false + + // Set up a cache where we expect load to never be called + $cache = $this->getCacheMock(); + $cache->expects($this->exactly(2)) + ->will($this->returnValue(false)) + ->method('load'); + + $manifest->expects($this->any()) + ->method('getCache') + ->will($this->returnValue($cache)); + + $manifest->__construct($base); + + // Test regeneration if 'variant_key_spec' cache file is missing + $manifest = $this->getManifestMock(array('getCache', 'regenerate', 'buildYamlConfigVariant')); + + $manifest->expects($this->once())// regenerate should be called once + ->method('regenerate') + ->with($this->equalTo(false)); // includeTests = false + + + $cache = $this->getCacheMock(); + $cache->expects($this->exactly(2)) + ->method('load') + ->will($this->returnCallback(function ($parameter) { + if (strpos($parameter, 'variant_key_spec') !== false) { + return false; + } + return array(); + })); + + $manifest->expects($this->any()) + ->method('getCache') + ->will($this->returnValue($cache)); + + $manifest->__construct($base); + + // Test regeneration if 'php_config_sources' cache file is missing + $manifest = $this->getManifestMock(array('getCache', 'regenerate', 'buildYamlConfigVariant')); + + $manifest->expects($this->once())// regenerate should be called once + ->method('regenerate') + ->with($this->equalTo(false)); // includeTests = false + + $cache = $this->getCacheMock(); + $cache->expects($this->exactly(2)) + ->method('load') + ->will($this->returnCallback(function ($parameter) { + if (strpos($parameter, 'php_config_sources') !== false) { + return false; + } + return array(); + })); + + $manifest->expects($this->any()) + ->method('getCache') + ->will($this->returnValue($cache)); + + $manifest->__construct($base); + } + /** * This test checks the processing of before and after reference paths (module-name/filename#fragment) * This method uses fixture/configmanifest/mysite/_config/addyamlconfigfile.yml as a fixture From 17d123a3be3a2c9e21845fda89c61f00301f78f5 Mon Sep 17 00:00:00 2001 From: Stephan Bauer Date: Mon, 16 Jan 2017 21:24:34 +0100 Subject: [PATCH 5/6] BUG Ensure correct regeneration of ConfigManifest if only one of the cache files is missing --- core/manifest/ConfigManifest.php | 2 +- tests/core/manifest/ConfigManifestTest.php | 77 ++++++++++++++++++++++ 2 files changed, 78 insertions(+), 1 deletion(-) diff --git a/core/manifest/ConfigManifest.php b/core/manifest/ConfigManifest.php index af04ae9e9..e9091bf6f 100644 --- a/core/manifest/ConfigManifest.php +++ b/core/manifest/ConfigManifest.php @@ -99,7 +99,7 @@ class SS_ConfigManifest { } // If we don't have a variantKeySpec (because we're forcing regen, or it just wasn't in the cache), generate it - if (false === $this->variantKeySpec) { + if (false === $this->phpConfigSources || false === $this->variantKeySpec) { $this->regenerate($includeTests); } diff --git a/tests/core/manifest/ConfigManifestTest.php b/tests/core/manifest/ConfigManifestTest.php index a1fac8424..5b14df254 100644 --- a/tests/core/manifest/ConfigManifestTest.php +++ b/tests/core/manifest/ConfigManifestTest.php @@ -152,6 +152,83 @@ class ConfigManifestTest extends SapphireTest { $manifest->__construct(dirname(__FILE__).'/fixtures/configmanifest', false, false); } + /** + * Test cache regeneration if all or some of the cache files are missing + * + * 1. Test regeneration if all cache files are missing + * 2. Test regeneration if 'variant_key_spec' cache file is missing + * 3. Test regeneration if 'php_config_sources' cache file is missing + */ + public function testAutomaticCacheRegeneration(){ + $base = dirname(__FILE__) . '/fixtures/configmanifest'; + + // Test regeneration if all cache files are missing + $manifest = $this->getManifestMock(array('getCache', 'regenerate', 'buildYamlConfigVariant')); + + $manifest->expects($this->once())// regenerate should be called once + ->method('regenerate') + ->with($this->equalTo(false)); // includeTests = false + + // Set up a cache where we expect load to never be called + $cache = $this->getCacheMock(); + $cache->expects($this->exactly(2)) + ->will($this->returnValue(false)) + ->method('load'); + + $manifest->expects($this->any()) + ->method('getCache') + ->will($this->returnValue($cache)); + + $manifest->__construct($base); + + // Test regeneration if 'variant_key_spec' cache file is missing + $manifest = $this->getManifestMock(array('getCache', 'regenerate', 'buildYamlConfigVariant')); + + $manifest->expects($this->once())// regenerate should be called once + ->method('regenerate') + ->with($this->equalTo(false)); // includeTests = false + + + $cache = $this->getCacheMock(); + $cache->expects($this->exactly(2)) + ->method('load') + ->will($this->returnCallback(function ($parameter) { + if (strpos($parameter, 'variant_key_spec') !== false) { + return false; + } + return array(); + })); + + $manifest->expects($this->any()) + ->method('getCache') + ->will($this->returnValue($cache)); + + $manifest->__construct($base); + + // Test regeneration if 'php_config_sources' cache file is missing + $manifest = $this->getManifestMock(array('getCache', 'regenerate', 'buildYamlConfigVariant')); + + $manifest->expects($this->once())// regenerate should be called once + ->method('regenerate') + ->with($this->equalTo(false)); // includeTests = false + + $cache = $this->getCacheMock(); + $cache->expects($this->exactly(2)) + ->method('load') + ->will($this->returnCallback(function ($parameter) { + if (strpos($parameter, 'php_config_sources') !== false) { + return false; + } + return array(); + })); + + $manifest->expects($this->any()) + ->method('getCache') + ->will($this->returnValue($cache)); + + $manifest->__construct($base); + } + /** * This test checks the processing of before and after reference paths (module-name/filename#fragment) * This method uses fixture/configmanifest/mysite/_config/addyamlconfigfile.yml as a fixture From 6057df320211a728cf7f99639b17f82cd0e6d1ae Mon Sep 17 00:00:00 2001 From: Chris Lock Date: Tue, 17 Jan 2017 09:54:18 +0000 Subject: [PATCH 6/6] Adding lots of PHPDoc --- email/Email.php | 67 ++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 66 insertions(+), 1 deletion(-) diff --git a/email/Email.php b/email/Email.php index b35dcbabe..d1d901dbd 100644 --- a/email/Email.php +++ b/email/Email.php @@ -149,7 +149,14 @@ class Email extends ViewableData { private static $cc_all_emails_to = null; /** - * Create a new email. + * Create a new email + * @param string $from + * @param string $to + * @param string $subject + * @param string $body + * @param string $bounceHandlerURL + * @param string $cc + * @param string $bcc */ public function __construct($from = null, $to = null, $subject = null, $body = null, $bounceHandlerURL = null, $cc = null, $bcc = null) { @@ -168,6 +175,12 @@ class Email extends ViewableData { parent::__construct(); } + /** + * @param string $data + * @param string $filename + * @param string $mimetype + * @return $this + */ public function attachFileFromString($data, $filename, $mimetype = null) { $this->attachments[] = array( 'contents' => $data, @@ -195,55 +208,97 @@ class Email extends ViewableData { return $this; } + /** + * @return string + */ public function Subject() { return $this->subject; } + /** + * @return string + */ public function Body() { return $this->body; } + /** + * @return string + */ public function To() { return $this->to; } + /** + * @return string + */ public function From() { return $this->from; } + /** + * @return string + */ public function Cc() { return $this->cc; } + /** + * @return string + */ public function Bcc() { return $this->bcc; } + /** + * @param string $val + * @return $this + */ public function setSubject($val) { $this->subject = $val; return $this; } + /** + * @param string $val + * @return $this + */ public function setBody($val) { $this->body = $val; return $this; } + /** + * @param string $val + * @return $this + */ public function setTo($val) { $this->to = $val; return $this; } + /** + * @param string $val + * @return $this + */ public function setFrom($val) { $this->from = $val; return $this; } + /** + * @param string $val + * @return $this + */ public function setCc($val) { $this->cc = $val; return $this; } + /** + * @param string $val + * @return $this + */ public function setBcc($val) { $this->bcc = $val; return $this; @@ -252,6 +307,7 @@ class Email extends ViewableData { /** * Set the "Reply-To" header with an email address. * @param string $email The email address of the "Reply-To" header + * @return $this */ public function replyTo($email) { $this->addCustomHeader('Reply-To', $email); @@ -264,6 +320,7 @@ class Email extends ViewableData { * * @param string $headerName * @param string $headerValue + * @return $this */ public function addCustomHeader($headerName, $headerValue) { if($headerName == 'Cc') $this->cc = $headerValue; @@ -275,6 +332,9 @@ class Email extends ViewableData { return $this; } + /** + * @return string + */ public function BaseURL() { return Director::absoluteBaseURL(); } @@ -298,6 +358,7 @@ class Email extends ViewableData { * Set template name (without *.ss extension). * * @param string $template + * @return $this */ public function setTemplate($template) { $this->ss_template = $template; @@ -330,6 +391,7 @@ class Email extends ViewableData { /** * Used by {@link SSViewer} templates to detect if we're rendering an email template rather than a page template + * @return bool */ public function IsEmail() { return true; @@ -338,6 +400,7 @@ class Email extends ViewableData { /** * Populate this email template with values. * This may be called many times. + * return $this */ public function populateTemplate($data) { if($this->template_data) { @@ -356,6 +419,7 @@ class Email extends ViewableData { * the template into body. Called before send() or debugSend() * $isPlain=true will cause the template to be ignored, otherwise the GenericEmail template will be used * and it won't be plain email :) + * @return $this */ protected function parseVariables($isPlain = false) { $origState = Config::inst()->get('SSViewer', 'source_file_comments'); @@ -390,6 +454,7 @@ class Email extends ViewableData { /** * Validates the email address. Returns true of false + * @return mixed */ public static function validEmailAddress($address) { if (function_exists('filter_var')) {