Improvements video embedding

- test case for all domains allowed - documentation in admin panel for domain parameter - more readable default after first installation
Schlaefer · May 11, 2012 · 8f2312e · 8f2312e
1 parent 2cc0f8e
commit 8f2312e
Show file tree

Hide file tree

Showing 6 changed files with 56 additions and 12 deletions.
diff --git 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!)."
diff --git 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
@@ -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
@@ -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
@@ -756,29 +756,31 @@ 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);'),
 							explode('|', self::$_allowedVideoDomains)
 					), 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
@@ -67,13 +67,13 @@
 		<?php foreach( $autoSettings as $k => $v ) : ?>
 		<tr>
 			<td>
-				<?php echo $k; ?>
+				<?php echo __d('nondynamic', $k); ?>
 			</td>
 			<td>
 				<?php echo $v; ?>
 			</td>
 			<td>
-				<p><?php echo __($k.'_exp'); ?></p>
+				<p><?php echo __d('nondynamic', $k.'_exp'); ?></p>
 			</td>
 			<td>
 				<?php echo $this->Html->link(