Loosen restrictions on TV captions and descriptions

Allow tags and attributes as defined in new system settings. Also, created a new stripHtml method on the php side to coordinate the rules when javascript is not applicable (e.g., when elements are created or updated programmatically).
modxcms · Jan 20, 2022 · d4355e8 · d4355e8
1 parent 7a7bb97
commit d4355e8
Show file tree

Hide file tree

Showing 12 changed files with 299 additions and 56 deletions.
diff --git a/_build/data/transport.core.system_settings.php b/_build/data/transport.core.system_settings.php
@@ -470,6 +470,42 @@
   'area' => 'site',
   'editedon' => null,
 ], '', true, true);
+$settings['elements_caption_allowedattr'] = $xpdo->newObject(modSystemSetting::class);
+$settings['elements_caption_allowedattr']->fromArray([
+  'key' => 'elements_caption_allowedattr',
+  'value' => 'href',
+  'xtype' => 'textfield',
+  'namespace' => 'core',
+  'area' => 'manager',
+  'editedon' => null,
+], '', true, true);
+$settings['elements_caption_allowedtags'] = $xpdo->newObject(modSystemSetting::class);
+$settings['elements_caption_allowedtags']->fromArray([
+  'key' => 'elements_caption_allowedtags',
+  'value' => 'a',
+  'xtype' => 'textfield',
+  'namespace' => 'core',
+  'area' => 'manager',
+  'editedon' => null,
+], '', true, true);
+$settings['elements_description_allowedattr'] = $xpdo->newObject(modSystemSetting::class);
+$settings['elements_description_allowedattr']->fromArray([
+  'key' => 'elements_description_allowedattr',
+  'value' => 'href,src,class',
+  'xtype' => 'textfield',
+  'namespace' => 'core',
+  'area' => 'manager',
+  'editedon' => null,
+], '', true, true);
+$settings['elements_description_allowedtags'] = $xpdo->newObject(modSystemSetting::class);
+$settings['elements_description_allowedtags']->fromArray([
+  'key' => 'elements_description_allowedtags',
+  'value' => 'div,p,ul,ol,li,img,span,br,strong,b,em,i,a',
+  'xtype' => 'textfield',
+  'namespace' => 'core',
+  'area' => 'manager',
+  'editedon' => null,
+], '', true, true);
 $settings['emailsender'] = $xpdo->newObject(modSystemSetting::class);
 $settings['emailsender']->fromArray([
   'key' => 'emailsender',

diff --git a/core/lexicon/en/setting.inc.php b/core/lexicon/en/setting.inc.php
@@ -241,6 +241,18 @@
 $_lang['setting_default_per_page'] = 'Default Per Page';
 $_lang['setting_default_per_page_desc'] = 'The default number of results to show in grids throughout the manager.';
 
+$_lang['setting_elements_caption_allowedattr'] = 'Element Captions: Allowed Attributes';
+$_lang['setting_elements_caption_allowedattr_desc'] = 'When adding an element caption, the html tag attribute(s) provided in this comma-separated list will be preserved. This currently only applies to template variables (TVs).';
+
+$_lang['setting_elements_caption_allowedtags'] = 'Element Captions: Allowed Tags';
+$_lang['setting_elements_caption_allowedtags_desc'] = 'When adding an element caption, the html tag(s) provided in this comma-separated list will be preserved. This currently only applies to template variables (TVs).';
+
+$_lang['setting_elements_description_allowedattr'] = 'Element Descriptions: Allowed Attributes';
+$_lang['setting_elements_description_allowedattr_desc'] = 'When adding an element description, the html tag attribute(s) provided in this comma-separated list will be preserved.';
+
+$_lang['setting_elements_description_allowedtags'] = 'Element Descriptions: Allowed Tags';
+$_lang['setting_elements_description_allowedtags_desc'] = 'When adding an element description, the html tag(s) provided in this comma-separated list will be preserved.';
+
 $_lang['setting_emailsender'] = 'Registration Email From Address';
 $_lang['setting_emailsender_desc'] = 'Here you can specify the email address used when sending Users their usernames and passwords.';
 $_lang['setting_emailsender_err'] = 'Please state the administration email address.';

diff --git a/core/src/Revolution/Processors/Element/Create.php b/core/src/Revolution/Processors/Element/Create.php
@@ -64,13 +64,26 @@ public function beforeSave()
         $elementClassName = array_pop(explode('\\', $this->classKey));
 
         if ($elementClassName === 'modTemplateVar') {
-            if ($caption = $this->getProperty('caption', '')) {
-                $this->object->set('caption', strip_tags($caption));
+            if ($caption = trim($this->getProperty('caption', ''))) {
+                $caption = $this->modx->stripHtml(
+                    $caption,
+                    $this->modx->getOption('elements_caption_allowedtags'),
+                    $this->modx->getOption('elements_caption_allowedattr')
+                );
+                $this->object->set('caption', $caption);
             }
         }
 
-        if ($description = $this->getProperty('description', '')) {
-            $this->object->set('description', strip_tags($description));
+        if ($description = trim($this->getProperty('description', ''))) {
+            $description = $elementClassName === 'modTemplateVar'
+                ? $this->modx->stripHtml(
+                    $description,
+                    $this->modx->getOption('elements_description_allowedtags'),
+                    $this->modx->getOption('elements_description_allowedattr')
+                )
+                : strip_tags($description)
+                ;
+            $this->object->set('description', $description);
         }
 
         $name = $this->getProperty($this->elementNameField, '');

diff --git a/core/src/Revolution/Processors/Element/Update.php b/core/src/Revolution/Processors/Element/Update.php
@@ -59,13 +59,26 @@ public function beforeSave()
         $elementClassName = array_pop(explode('\\', $this->classKey));
 
         if ($elementClassName === 'modTemplateVar') {
-            if ($caption = $this->getProperty('caption', '')) {
-                $this->object->set('caption', strip_tags($caption));
+            if ($caption = trim($this->getProperty('caption', ''))) {
+                $caption = $this->modx->stripHtml(
+                    $caption,
+                    $this->modx->getOption('elements_caption_allowedtags'),
+                    $this->modx->getOption('elements_caption_allowedattr')
+                );
+                $this->object->set('caption', $caption);
             }
         }
 
-        if ($description = $this->getProperty('description', '')) {
-            $this->object->set('description', strip_tags($description));
+        if ($description = trim($this->getProperty('description', ''))) {
+            $description = $elementClassName === 'modTemplateVar'
+                ? $this->modx->stripHtml(
+                    $description,
+                    $this->modx->getOption('elements_description_allowedtags'),
+                    $this->modx->getOption('elements_description_allowedattr')
+                )
+                : strip_tags($description)
+                ;
+            $this->object->set('description', $description);
         }
 
         /* verify element has a name and that name does not already exist */

diff --git a/core/src/Revolution/Processors/Security/Forms/Set/Update.php b/core/src/Revolution/Processors/Security/Forms/Set/Update.php
@@ -1,4 +1,5 @@
 <?php
+
 /*
  * This file is part of MODX Revolution.
  *
@@ -121,7 +122,11 @@ public function setFieldRules()
                 $this->newRules[] = $rule;
             }
             if (!empty($field['label'])) {
-                $field['label'] = strip_tags($field['label']);
+                $field['label'] = $this->modx->stripHtml(
+                    $field['label'],
+                    $this->modx->getOption('elements_caption_allowedtags'),
+                    $this->modx->getOption('elements_caption_allowedattr')
+                );
                 $rule = $this->modx->newObject(modActionDom::class);
                 $rule->set('set', $this->object->get('id'));
                 $rule->set('action', $this->object->get('action'));
@@ -287,7 +292,11 @@ public function setTVRules()
                 $this->newRules[] = $rule;
             }
             if (!empty($tvData['label'])) {
-                $tvData['label'] = strip_tags($tvData['label']);
+                $tvData['label'] = $this->modx->stripHtml(
+                    $tvData['label'],
+                    $this->modx->getOption('elements_caption_allowedtags'),
+                    $this->modx->getOption('elements_caption_allowedattr')
+                );
                 $rule = $this->modx->newObject(modActionDom::class);
                 $rule->set('set', $this->object->get('id'));
                 $rule->set('action', $this->object->get('action'));

diff --git a/core/src/Revolution/modTemplateVar.php b/core/src/Revolution/modTemplateVar.php
@@ -75,7 +75,7 @@ class modTemplateVar extends modElement
      *
      * {@inheritdoc}
      */
-    function __construct(& $xpdo)
+    public function __construct(&$xpdo)
     {
         parent:: __construct($xpdo);
         $this->setToken('*');
@@ -161,7 +161,7 @@ public function process($properties = null, $content = null)
             /* copy the content source to the output buffer */
             $this->_output = $this->_content;
 
-            if (is_string($this->_output) && !empty ($this->_output)) {
+            if (is_string($this->_output) && !empty($this->_output)) {
                 /* turn the processed properties into placeholders */
                 $scope = $this->xpdo->toPlaceholders($this->_properties, '', '.', true);
 
@@ -217,10 +217,10 @@ public function getValue($resourceId = 0)
         $value = null;
         $resourceId = intval($resourceId);
         if ($resourceId) {
-            if (is_object($this->xpdo->resource) && $resourceId === (integer)$this->xpdo->resourceIdentifier && is_array($this->xpdo->resource->get($this->get('name')))) {
+            if (is_object($this->xpdo->resource) && $resourceId === (int)$this->xpdo->resourceIdentifier && is_array($this->xpdo->resource->get($this->get('name')))) {
                 $valueArray = $this->xpdo->resource->get($this->get('name'));
                 $value = $valueArray[1];
-            } elseif ($resourceId === (integer)$this->get('resourceId') && array_key_exists('value', $this->_fields)) {
+            } elseif ($resourceId === (int)$this->get('resourceId') && array_key_exists('value', $this->_fields)) {
                 $value = $this->get('value');
             } else {
                 $resource = $this->xpdo->getObject(modTemplateVarResource::class, [
@@ -269,8 +269,7 @@ public function setValue($resourceId = 0, $value = null)
                     $templateVarResource->set('value', $value);
                 }
                 $this->addMany($templateVarResource);
-            } elseif (!$templateVarResource->isNew()
-                && ($value === null || $value === $this->get('default_text'))) {
+            } elseif (!$templateVarResource->isNew() && ($value === null || $value === $this->get('default_text'))) {
                 $templateVarResource->remove();
             }
         }
@@ -325,8 +324,10 @@ public function prepareOutput($value, $resourceId = 0)
         $mTypes = $this->xpdo->getOption('manipulatable_url_tv_output_types', null, 'image,file');
         $mTypes = explode(',', $mTypes);
         if (!empty($value) && in_array($this->get('type'), $mTypes)) {
-            $context = !empty($resourceId) ? $this->xpdo->getObject(modResource::class,
-                $resourceId)->get('context_key') : $this->xpdo->context->get('key');
+            $context = !empty($resourceId)
+                ? $this->xpdo->getObject(modResource::class, $resourceId)->get('context_key')
+                : $this->xpdo->context->get('key')
+                ;
             $sourceCache = $this->getSourceCache($context);
             $classKey = $sourceCache['class_key'];
             if (!empty($sourceCache) && !empty($classKey)) {
@@ -336,8 +337,7 @@ public function prepareOutput($value, $resourceId = 0)
                     if ($source) {
                         $source->fromArray($sourceCache, '', true, true);
                         $source->initialize();
-                        $isAbsolute = strpos($value, 'http://') === 0 || strpos($value,
-                                'https://') === 0 || strpos($value, 'ftp://') === 0;
+                        $isAbsolute = strpos($value, 'http://') === 0 || strpos($value, 'https://') === 0 || strpos($value, 'ftp://') === 0;
                         if (!$isAbsolute) {
                             $value = $source->prepareOutputUrl($value);
                         }
@@ -380,8 +380,11 @@ public function renderInput($resource = null, $options = [])
         }
         if (!isset($this->xpdo->smarty)) {
             $this->xpdo->getService('smarty', modSmarty::class, '', [
-                'template_dir' => $this->xpdo->getOption('manager_path') . 'templates/' . $this->xpdo->getOption('manager_theme',
-                        null, 'default') . '/',
+                'template_dir' => $this->xpdo->getOption('manager_path') . 'templates/' . $this->xpdo->getOption(
+                    'manager_theme',
+                    null,
+                    'default'
+                ) . '/'
             ]);
         }
         $this->xpdo->smarty->assign('style', $style);
@@ -402,8 +405,12 @@ public function renderInput($resource = null, $options = [])
         $this->set('processedValue', $value);
         $this->set('default_text', $this->processBindings($this->get('default_text'), $resourceId));
 
-        /* strip tags from description */
-        // $this->set('description', strip_tags($this->get('description')));
+        /* remove disallowed tags and attributes from description */
+        $this->set('description', $this->modx->stripHtml(
+            $this->get('description'),
+            $this->modx->getOption('elements_description_allowedtags'),
+            $this->modx->getOption('elements_description_allowedattr')
+        ));
 
         $params = [];
         if ($paramstring = $this->get('display_params')) {
@@ -627,8 +634,7 @@ public function checkForFormCustomizationRules($value, &$resource)
             $c = $this->xpdo->newQuery(modActionDom::class);
             $c->innerJoin(modFormCustomizationSet::class, 'FCSet');
             $c->innerJoin(modFormCustomizationProfile::class, 'Profile', 'FCSet.profile = Profile.id');
-            $c->leftJoin(modFormCustomizationProfileUserGroup::class, 'ProfileUserGroup',
-                'Profile.id = ProfileUserGroup.profile');
+            $c->leftJoin(modFormCustomizationProfileUserGroup::class, 'ProfileUserGroup', 'Profile.id = ProfileUserGroup.profile');
             $c->leftJoin(modFormCustomizationProfile::class, 'UGProfile', 'UGProfile.id = ProfileUserGroup.profile');
             $ruleFieldName = $this->xpdo->escape('rule');
             $c->where([
@@ -654,8 +660,7 @@ public function checkForFormCustomizationRules($value, &$resource)
                 ], xPDOQuery::SQL_AND, null, 2);
             }
             if (!empty($this->xpdo->request) && !empty($this->xpdo->request->action)) {
-                $wildAction = substr($this->xpdo->request->action, 0,
-                        strrpos($this->xpdo->request->action, '/')) . '/*';
+                $wildAction = substr($this->xpdo->request->action, 0, strrpos($this->xpdo->request->action, '/')) . '/*';
                 $c->where([
                     'modActionDom.action:IN' => [$this->xpdo->request->action, $wildAction],
                 ]);
@@ -896,7 +901,7 @@ public function processBindings($value = '', $resourceId = 0, $preProcess = true
             case 'DOCUMENT': /* retrieve a document and process it's content */
                 if ($preProcess) {
                     $query = $this->xpdo->newQuery(modResource::class, [
-                        'id' => (integer)$param,
+                        'id' => (int)$param,
                         'deleted' => false,
                     ]);
                     $query->select('content');
@@ -917,8 +922,10 @@ public function processBindings($value = '', $resourceId = 0, $preProcess = true
                     $dbtags['DBASE'] = $this->xpdo->getOption('dbname');
                     $dbtags['PREFIX'] = $this->xpdo->getOption('table_prefix');
                     foreach ($dbtags as $key => $pValue) {
-                        if (!is_scalar($pValue)) continue;
-                        $param = str_replace('[[+'.$key.']]', (string)$pValue, $param);
+                        if (!is_scalar($pValue)) {
+                            continue;
+                        }
+                        $param = str_replace('[[+' . $key . ']]', (string)$pValue, $param);
                     }
                     $stmt = $this->xpdo->query('SELECT ' . $param);
                     if ($stmt && $stmt instanceof PDOStatement) {
@@ -973,7 +980,6 @@ public function processBindings($value = '', $resourceId = 0, $preProcess = true
             default:
                 $output = $value;
                 break;
-
         }
 
         /* support for nested bindings */
@@ -1003,11 +1009,11 @@ public function parseBinding($binding_string)
             $regexp2 = '/(\S+)\s+(.+)/is'; /* Split binding on second whitespace to get properties */
 
             $properties = [];
-            if (preg_match($regexp2, $match[2] , $match2)) {
+            if (preg_match($regexp2, $match[2], $match2)) {
                 if (isset($match2[2])) {
-                    $props = json_decode($match2[2],true);
+                    $props = json_decode($match2[2], true);
                     $valid = json_last_error() === JSON_ERROR_NONE;
-                    if ($valid && is_array($props)){
+                    if ($valid && is_array($props)) {
                         $properties = $props;
                         $match[2] = $match2[1];
                     } else {
@@ -1039,8 +1045,10 @@ public function processInheritBinding($default = '', $resourceId = null)
         $output = $default; /* Default to param value if no content from parents */
         $resource = null;
         $resourceColumns = $this->xpdo->getSelectColumns(modResource::class, '', '', ['id', 'parent']);
-        $resourceQuery = new xPDOCriteria($this->xpdo,
-            "SELECT {$resourceColumns} FROM {$this->xpdo->getTableName(modResource::class)} WHERE id = ?");
+        $resourceQuery = new xPDOCriteria(
+            $this->xpdo,
+            "SELECT {$resourceColumns} FROM {$this->xpdo->getTableName(modResource::class)} WHERE id = ?"
+        );
         if (!empty($resourceId) && (!($this->xpdo->resource instanceof modResource) || $this->xpdo->resource->get('id') != $resourceId)) {
             if ($resourceQuery->stmt && $resourceQuery->stmt->execute([$resourceId])) {
                 $result = $resourceQuery->stmt->fetchAll(PDO::FETCH_ASSOC);
@@ -1113,11 +1121,11 @@ public function findPolicy($context = '')
         $policy = [];
         $context = !empty($context) ? $context : $this->xpdo->context->get('key');
         if ($context === $this->xpdo->context->get('key')) {
-            $catEnabled = (boolean)$this->xpdo->getOption('access_category_enabled', null, true);
-            $rgEnabled = (boolean)$this->xpdo->getOption('access_resource_group_enabled', null, true);
+            $catEnabled = (bool)$this->xpdo->getOption('access_category_enabled', null, true);
+            $rgEnabled = (bool)$this->xpdo->getOption('access_resource_group_enabled', null, true);
         } elseif ($this->xpdo->getContext($context)) {
-            $catEnabled = (boolean)$this->xpdo->contexts[$context]->getOption('access_category_enabled', true);
-            $rgEnabled = (boolean)$this->xpdo->contexts[$context]->getOption('access_resource_group_enabled', true);
+            $catEnabled = (bool)$this->xpdo->contexts[$context]->getOption('access_category_enabled', true);
+            $rgEnabled = (bool)$this->xpdo->contexts[$context]->getOption('access_resource_group_enabled', true);
         }
         $enabled = ($catEnabled || $rgEnabled);
         if ($enabled) {