From 8f2312e4175de4a44d26b8890be44e637f555833 Mon Sep 17 00:00:00 2001 From: Schlaefer Date: Fri, 11 May 2012 11:41:14 +0200 Subject: [PATCH] Improvements video embedding - test case for all domains allowed - documentation in admin panel for domain parameter - more readable default after first installation --- app/Locale/deu/LC_MESSAGES/nondynamic.po | 8 +++++ app/Locale/eng/LC_MESSAGES/nondynamic.po | 9 +++-- .../Install/Config/Data/SettingData.php | 2 +- .../Case/View/Helper/BbcodeHelperTest.php | 35 +++++++++++++++++-- app/View/Helper/BbcodeHelper.php | 10 +++--- app/View/Settings/admin_index.ctp | 4 +-- 6 files changed, 56 insertions(+), 12 deletions(-) diff --git a/app/Locale/deu/LC_MESSAGES/nondynamic.po b/app/Locale/deu/LC_MESSAGES/nondynamic.po index 25224b946..3eb66a3b2 100644 --- a/app/Locale/deu/LC_MESSAGES/nondynamic.po +++ b/app/Locale/deu/LC_MESSAGES/nondynamic.po @@ -43,3 +43,11 @@ msgstr "Kategorie nur für eingeloggte Nutzer sichtbar" # msgid "category_acs_2_exp" msgstr "Kategorie nur für Moderatoren sichtbar" + +# +msgid "video_domains_allowed" +msgstr "Einbettbare (Video-) Quellen" + +# +msgid "video_domains_allowed_exp" +msgstr "Domainnamen einbindbar für Video und iFrame. Ein Asterisk * bedeuted freies Einbinden (Vorsicht, dies kann ebenfalls zum Einbinden von Schadsoftware genutzt werden!)." \ No newline at end of file diff --git a/app/Locale/eng/LC_MESSAGES/nondynamic.po b/app/Locale/eng/LC_MESSAGES/nondynamic.po index de3d82539..83bb215b9 100644 --- a/app/Locale/eng/LC_MESSAGES/nondynamic.po +++ b/app/Locale/eng/LC_MESSAGES/nondynamic.po @@ -43,7 +43,10 @@ msgstr "Category only visible for logged in users" # msgid "category_acs_2_exp" msgstr "Category only visible for moderators" +# +msgid "video_domains_allowed" +msgstr "Embeddable Video Sources" - - - +# +msgid "video_domains_allowed_exp" +msgstr "Domain names embeddable for video/iframe. Asterisk * means all sources allowed (This could pose a security risk if malicious code is embedded!)." diff --git a/app/Plugin/Install/Config/Data/SettingData.php b/app/Plugin/Install/Config/Data/SettingData.php index 9691276aa..10583c018 100755 --- a/app/Plugin/Install/Config/Data/SettingData.php +++ b/app/Plugin/Install/Config/Data/SettingData.php @@ -23,7 +23,7 @@ class SettingData { ), array( 'name' => 'video_domains_allowed', - 'value' => 'youtube|vimeo', + 'value' => 'youtube | vimeo', ), array( 'name' => 'flattr_category', diff --git a/app/Test/Case/View/Helper/BbcodeHelperTest.php b/app/Test/Case/View/Helper/BbcodeHelperTest.php index ec0be9490..a19717da4 100755 --- a/app/Test/Case/View/Helper/BbcodeHelperTest.php +++ b/app/Test/Case/View/Helper/BbcodeHelperTest.php @@ -374,8 +374,39 @@ function testIframe() { $result = $this->Bbcode->parse($input); $this->assertNoPattern($expected, $result); - Configure::write('Saito.Settings.bbcode_img', $bbcode_img); - Configure::write('Saito.Settings.video_domains_allowed', $video_domains); + /* + * test if all domains are allowed + */ + Configure::write('Saito.Settings.video_domains_allowed', '*'); + + $refObject = new ReflectionObject($this->Bbcode); + $refProperty = $refObject->getProperty('_allowedVideoDomains'); + $refProperty->setAccessible(true); + $refProperty->setValue('_allowedVideoDomains', null); + + $input = '[iframe height=349 width=560 ' . + 'src=http://www.youtubescam.com/embed/HdoW3t_WorU ' . + '][/iframe]'; + $expected = 'src="http://www.youtubescam.com/embed/HdoW3t_WorU'; + $result = $this->Bbcode->parse($input); + $this->assertContains($expected, $result); + + /* + * test if no domains are allowed + */ + Configure::write('Saito.Settings.video_domains_allowed', ''); + + $refObject = new ReflectionObject($this->Bbcode); + $refProperty = $refObject->getProperty('_allowedVideoDomains'); + $refProperty->setAccessible(true); + $refProperty->setValue('_allowedVideoDomains', null); + + $input = '[iframe height=349 width=560 ' . + 'src=http://www.youtubescam.com/embed/HdoW3t_WorU ' . + '][/iframe]'; + $expected = '/src/i'; + $result = $this->Bbcode->parse($input); + $this->assertNoPattern($expected, $result); } function testExternalImage() { diff --git a/app/View/Helper/BbcodeHelper.php b/app/View/Helper/BbcodeHelper.php index 88d3f368f..1967caa51 100644 --- a/app/View/Helper/BbcodeHelper.php +++ b/app/View/Helper/BbcodeHelper.php @@ -756,16 +756,18 @@ protected static function _checkAndAddProtocol($string) { /** * Checks if an domain is allowed by comparing it to the admin preference - * + * * @param string $src * @return bool */ protected function _isVideoDomainAllowed($src) { self::$_videoErrorMessage->reset(); - //* initialy setup self::$_allowedVideoDomains + // initialy setup self::$_allowedVideoDomains if ( self::$_allowedVideoDomains === null ): + // @td bad mvc; set allowed domains when initizlizing helper in controller self::$_allowedVideoDomains = trim(Configure::read('Saito.Settings.video_domains_allowed')); + // @td bad mvc; the array should be created by the Settings Model self::$_allowedVideoDomains = array_fill_keys( array_map( create_function('$in', 'return trim($in);'), @@ -773,12 +775,12 @@ protected function _isVideoDomainAllowed($src) { ), 1); endif; - //* `*` admin pref allows all domains + // `*` admin pref allows all domains if ( self::$_allowedVideoDomains === array( '*' => 1 ) ): return true; endif; - //* check if particular domain is allowed + // check if particular domain is allowed $host = self::_getDomainForUri($src); if ( isset(self::$_allowedVideoDomains[$host]) === true ): return true; diff --git a/app/View/Settings/admin_index.ctp b/app/View/Settings/admin_index.ctp index 786d0b3be..6adff29fc 100644 --- a/app/View/Settings/admin_index.ctp +++ b/app/View/Settings/admin_index.ctp @@ -67,13 +67,13 @@ $v ) : ?> - + -

+

Html->link(