From 3a5b726fc0082d2a6b304ac33bec492d3802e6c5 Mon Sep 17 00:00:00 2001 From: Jim Graham Date: Wed, 8 Dec 2021 11:38:05 -0500 Subject: [PATCH 1/9] Two minor lexicon fixes --- core/lexicon/en/chunk.inc.php | 2 +- core/lexicon/en/tv.inc.php | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/core/lexicon/en/chunk.inc.php b/core/lexicon/en/chunk.inc.php index 9ffddaf3127..c0d44861c74 100644 --- a/core/lexicon/en/chunk.inc.php +++ b/core/lexicon/en/chunk.inc.php @@ -26,7 +26,7 @@ $_lang['chunk_err_nf'] = 'Chunk not found!'; $_lang['chunk_err_nfs'] = 'Chunk not found with id: [[+id]]'; $_lang['chunk_err_ns'] = 'Chunk not specified.'; -$_lang['chunk_err_ns_name'] = 'Please specify a name.'; +$_lang['chunk_err_ns_name'] = 'Please specify a name for this chunk.'; $_lang['chunk_lock'] = 'Lock chunk for editing'; $_lang['chunk_lock_desc'] = 'Only users with “edit_locked” permissions can edit this Chunk.'; $_lang['chunk_name_desc'] = 'Place the content generated by this Chunk in a Resource, Template, or other Chunk using the following MODX tag: [[+tag]]'; diff --git a/core/lexicon/en/tv.inc.php b/core/lexicon/en/tv.inc.php index a1b71f0018b..257514d354d 100644 --- a/core/lexicon/en/tv.inc.php +++ b/core/lexicon/en/tv.inc.php @@ -33,6 +33,7 @@ $_lang['tv_err_nf'] = 'TV not found.'; $_lang['tv_err_nfs'] = 'TV not found with key: [[+id]]'; $_lang['tv_err_ns'] = 'TV not specified.'; +$_lang['tv_err_ns_name'] = 'Please specify a name for this TV.'; $_lang['tv_err_reserved_name'] = 'A TV cannot have the same name as a Resource field.'; $_lang['tv_err_save_access_permissions'] = 'An error occured while attempting to save TV access permissions.'; $_lang['tv_err_save'] = 'An error occurred while saving the TV.'; From 1fdac97739d913d35b41bc4c0addf251c8d18bde Mon Sep 17 00:00:00 2001 From: Jim Graham Date: Wed, 8 Dec 2021 11:39:58 -0500 Subject: [PATCH 2/9] Processor updates Prevents saving of tags in fields that will be displayed in the interface (name, caption, description) --- .../Revolution/Processors/Element/Create.php | 65 ++++++++++++------- .../Revolution/Processors/Element/Update.php | 48 ++++++++++---- .../Processors/Security/Forms/Set/Update.php | 3 + core/src/Revolution/modTemplateVar.php | 2 +- 4 files changed, 82 insertions(+), 36 deletions(-) diff --git a/core/src/Revolution/Processors/Element/Create.php b/core/src/Revolution/Processors/Element/Create.php index 59cbf46991f..f188a407420 100644 --- a/core/src/Revolution/Processors/Element/Create.php +++ b/core/src/Revolution/Processors/Element/Create.php @@ -29,6 +29,16 @@ abstract class Create extends CreateProcessor /** @var modElement $object */ public $object; + protected $elementNameField = 'name'; + + public function initialize() + { + if ($this->classKey === modTemplate::class) { + $this->elementNameField = 'templatename'; + } + return parent::initialize(); + } + /** * Cleanup the process and send back the response * @@ -37,9 +47,7 @@ abstract class Create extends CreateProcessor public function cleanup() { $this->clearCache(); - $fields = ['id', 'description', 'locked', 'category']; - array_push($fields, ($this->classKey == modTemplate::class ? 'templatename' : 'name')); - + $fields = ['id', $this->elementNameField, 'description', 'locked', 'category']; return $this->success('', $this->object->get($fields)); } @@ -50,14 +58,34 @@ public function cleanup() */ public function beforeSave() { - $nameField = $this->classKey === modTemplate::class ? 'templatename' : 'name'; - $name = $this->getProperty($nameField, ''); - - /* verify element with that name does not already exist */ - if ($this->alreadyExists($name)) { - $this->addFieldError($nameField, $this->modx->lexicon($this->objectType . '_err_ae', [ - 'name' => $name, - ])); + $locked = (bool)$this->getProperty('locked', false); + $this->object->set('locked', $locked); + + $elementClassName = array_pop(explode('\\', $this->classKey)); + + if ($elementClassName === 'modTemplateVar') { + if ($caption = $this->getProperty('caption', '')) { + $this->object->set('caption', strip_tags($caption)); + } + } + + if ($description = $this->getProperty('description', '')) { + $this->object->set('description', strip_tags($description)); + } + + $name = $this->getProperty($this->elementNameField, ''); + + /* verify element has a name and that name does not already exist */ + + if (empty($name)) { + $this->addFieldError($this->elementNameField, $this->modx->lexicon($this->objectType . '_err_ns_name')); + } else { + if ($this->alreadyExists($name)) { + $this->modx->error->addField( + $this->elementNameField, + $this->modx->lexicon($this->objectType . '_err_ae', ['name' => $name]) + ); + } } $category = $this->getProperty('category', 0); @@ -72,9 +100,6 @@ public function beforeSave() } } - $locked = (bool)$this->getProperty('locked', false); - $this->object->set('locked', $locked); - $this->setElementProperties(); $this->validateElement(); @@ -90,7 +115,7 @@ public function beforeSave() } /** - * Check to see if a Chunk already exists with specified name + * Check to see if an Element with the specified name already exists * * @param string $name * @@ -98,13 +123,9 @@ public function beforeSave() */ public function alreadyExists($name) { - if ($this->classKey == modTemplate::class) { - $c = ['templatename' => $name]; - } else { - $c = ['name' => $name]; - } - - return $this->modx->getCount($this->classKey, $c) > 0; + return $this->modx->getCount($this->classKey, [ + $this->elementNameField => $name, + ]) > 0; } /** diff --git a/core/src/Revolution/Processors/Element/Update.php b/core/src/Revolution/Processors/Element/Update.php index 81b424c5589..075e0923280 100644 --- a/core/src/Revolution/Processors/Element/Update.php +++ b/core/src/Revolution/Processors/Element/Update.php @@ -29,6 +29,16 @@ abstract class Update extends UpdateProcessor /** @var modElement $object */ public $object; + protected $elementNameField = 'name'; + + public function initialize() + { + if ($this->classKey === modTemplate::class) { + $this->elementNameField = 'templatename'; + } + return parent::initialize(); + } + public function beforeSet() { // Make sure the element isn't locked @@ -46,13 +56,25 @@ public function beforeSave() $this->object->set('locked', (bool)$locked); } - /* make sure a name was specified */ - $nameField = $this->classKey === modTemplate::class ? 'templatename' : 'name'; - $name = $this->getProperty($nameField, ''); + $elementClassName = array_pop(explode('\\', $this->classKey)); + + if ($elementClassName === 'modTemplateVar') { + if ($caption = $this->getProperty('caption', '')) { + $this->object->set('caption', strip_tags($caption)); + } + } + + if ($description = $this->getProperty('description', '')) { + $this->object->set('description', strip_tags($description)); + } + + /* verify element has a name and that name does not already exist */ + + $name = $this->getProperty($this->elementNameField, ''); if (empty($name)) { $this->addFieldError($nameField, $this->modx->lexicon($this->objectType . '_err_ns_name')); - } elseif ($this->alreadyExists($name)) { + } else if ($this->alreadyExists($name)) { /* if changing name, but new one already exists */ $this->modx->error->addField( $nameField, @@ -74,7 +96,7 @@ public function beforeSave() if ($this->object->staticContentChanged()) { if (!$this->object->isStaticSourceMutable()) { $this->addFieldError('static_file', $this->modx->lexicon('element_static_source_immutable')); - } elseif (!$this->object->isStaticSourceValidPath()) { + } else if (!$this->object->isStaticSourceValidPath()) { $this->addFieldError('static_file', $this->modx->lexicon('element_static_source_protected_invalid')); } } @@ -84,12 +106,10 @@ public function beforeSave() public function alreadyExists($name) { - $nameField = $this->classKey === modTemplate::class ? 'templatename' : 'name'; - return $this->modx->getCount($this->classKey, [ - 'id:!=' => $this->object->get('id'), - $nameField => $name, - ]) > 0; + 'id:!=' => $this->object->get('id'), + $this->elementNameField => $name, + ]) > 0; } public function afterSave() @@ -101,11 +121,13 @@ public function afterSave() public function cleanup() { - $fields = array('id', 'description', 'locked', 'category', 'content'); - array_push($fields, ($this->classKey == modTemplate::class ? 'templatename' : 'name')); + $fields = ['id', $this->elementNameField, 'description', 'locked', 'category', 'content']; return $this->success( '', - array_merge($this->object->get($fields), ['previous_category' => $this->previousCategory]) + array_merge( + $this->object->get($fields), + ['previous_category' => $this->previousCategory] + ) ); } } diff --git a/core/src/Revolution/Processors/Security/Forms/Set/Update.php b/core/src/Revolution/Processors/Security/Forms/Set/Update.php index 0a419c64b25..88f67b6aabb 100644 --- a/core/src/Revolution/Processors/Security/Forms/Set/Update.php +++ b/core/src/Revolution/Processors/Security/Forms/Set/Update.php @@ -121,6 +121,7 @@ public function setFieldRules() $this->newRules[] = $rule; } if (!empty($field['label'])) { + $field['label'] = strip_tags($field['label']); $rule = $this->modx->newObject(modActionDom::class); $rule->set('set', $this->object->get('id')); $rule->set('action', $this->object->get('action')); @@ -224,6 +225,7 @@ public function setTabRules() $this->newRules[] = $rule; } if (!empty($tab['label'])) { + $tab['label'] = strip_tags($tab['label']); $rule = $this->modx->newObject(modActionDom::class); $rule->set('set', $this->object->get('id')); $rule->set('action', $this->object->get('action')); @@ -285,6 +287,7 @@ public function setTVRules() $this->newRules[] = $rule; } if (!empty($tvData['label'])) { + $tvData['label'] = strip_tags($tvData['label']); $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 index aa76ea78d73..8149f39ef5f 100644 --- a/core/src/Revolution/modTemplateVar.php +++ b/core/src/Revolution/modTemplateVar.php @@ -403,7 +403,7 @@ public function renderInput($resource = null, $options = []) $this->set('default_text', $this->processBindings($this->get('default_text'), $resourceId)); /* strip tags from description */ - $this->set('description', strip_tags($this->get('description'))); + // $this->set('description', strip_tags($this->get('description'))); $params = []; if ($paramstring = $this->get('display_params')) { From f696056ba5883f5b60828c18484dcac31a59ab3c Mon Sep 17 00:00:00 2001 From: Jim Graham Date: Wed, 8 Dec 2021 11:42:26 -0500 Subject: [PATCH 3/9] modExt updates Cleans name, caption, and description fields on the front end (js) side before they are submitted to processors. --- manager/assets/modext/util/utilities.js | 21 +++++ .../modext/widgets/element/modx.panel.tv.js | 21 +++++ .../modext/widgets/fc/modx.panel.fcset.js | 27 ++++++- manager/assets/modext/widgets/windows.js | 79 +++++++++++++++++++ 4 files changed, 145 insertions(+), 3 deletions(-) diff --git a/manager/assets/modext/util/utilities.js b/manager/assets/modext/util/utilities.js index f4c456331cd..86cd021cb49 100644 --- a/manager/assets/modext/util/utilities.js +++ b/manager/assets/modext/util/utilities.js @@ -168,6 +168,27 @@ MODx.util.safeHtml = function (input, allowedTags, allowedAttributes) { return input.replace(eventAttributes, 'on​$1'); }; +/** + * Cleans and resets or returns a field's value; typically called: + * + * 1] via an event in a form field component's listeners object (use onChange callback) + * 2] via an event in a grid's column model (use onColumnRender callback) + * 3] directly via run + */ +MODx.util.stripAndEncode = { + onChange: function(cmp, newVal, originalVal) { + const value = cmp.getValue(); + cmp.setValue(MODx.util.stripAndEncode.run(value)); + }, + onColumnRender: function(value, metaData, record, rowIndex, colIndex) { + return MODx.util.stripAndEncode.run(value); + }, + run: function(value) { + value = Ext.util.Format.stripTags(value).replace(/\s{2,}/g, ' '); + return Ext.util.Format.htmlEncode(value); + } +} + /**************************************************************************** * Ext-specific overrides/extensions * ****************************************************************************/ diff --git a/manager/assets/modext/widgets/element/modx.panel.tv.js b/manager/assets/modext/widgets/element/modx.panel.tv.js index e1bb9d4b589..fcc4a097b7e 100644 --- a/manager/assets/modext/widgets/element/modx.panel.tv.js +++ b/manager/assets/modext/widgets/element/modx.panel.tv.js @@ -86,6 +86,7 @@ MODx.panel.TV = function(config) { ,maxLength: 50 ,enableKeyEvents: true ,allowBlank: false + ,blankText: _('tv_err_ns_name') ,value: config.record.name ,tabIndex: 1 ,listeners: { @@ -98,6 +99,16 @@ MODx.panel.TV = function(config) { MODx.setStaticElementPath('tv'); } ,scope: this + }, + change: { + fn: MODx.util.stripAndEncode.onChange + }, + blur: { + fn: function(cmp) { + if (!cmp.getValue()) { + cmp.markInvalid(_('tv_err_ns_name')); + } + } } } },{ @@ -174,6 +185,11 @@ MODx.panel.TV = function(config) { ,id: 'modx-tv-caption' ,tabIndex: 3 ,value: config.record.caption + ,listeners: { + change: { + fn: MODx.util.stripAndEncode.onChange + } + } },{ xtype: MODx.expandHelp ? 'label' : 'hidden' ,forId: 'modx-tv-caption' @@ -236,6 +252,11 @@ MODx.panel.TV = function(config) { ,maxLength: 255 ,tabIndex: 5 ,value: config.record.description || '' + ,listeners: { + change: { + fn: MODx.util.stripAndEncode.onChange + } + } },{ xtype: MODx.expandHelp ? 'label' : 'hidden' ,forId: 'modx-tv-description' diff --git a/manager/assets/modext/widgets/fc/modx.panel.fcset.js b/manager/assets/modext/widgets/fc/modx.panel.fcset.js index b38c2131b0d..771fd9a1269 100644 --- a/manager/assets/modext/widgets/fc/modx.panel.fcset.js +++ b/manager/assets/modext/widgets/fc/modx.panel.fcset.js @@ -264,9 +264,6 @@ MODx.grid.FCSetFields = function(config) { header: _('label') ,dataIndex: 'label' ,editor: { xtype: 'textfield' } - ,renderer: function(v,md) { - return Ext.util.Format.htmlEncode(v); - } },{ header: _('default_value') ,dataIndex: 'default_value' @@ -288,6 +285,14 @@ MODx.grid.FCSetFields = function(config) { }); MODx.grid.FCSetFields.superclass.constructor.call(this,config); this.propRecord = Ext.data.Record.create(config.fields); + this.on('afteredit', function(e) { + if (e.field === 'label') { + const value = MODx.util.stripAndEncode.run(e.value); + e.record.set('label', value); + e.record.commit(); + } + + }); }; Ext.extend(MODx.grid.FCSetFields,MODx.grid.LocalGrid); Ext.reg('modx-grid-fc-set-fields',MODx.grid.FCSetFields); @@ -341,6 +346,14 @@ MODx.grid.FCSetTabs = function(config) { }); MODx.grid.FCSetTabs.superclass.constructor.call(this,config); this.propRecord = Ext.data.Record.create(config.fields); + this.on('afteredit', function(e) { + if (e.field === 'label') { + const value = MODx.util.stripAndEncode.run(e.value); + e.record.set('label', value); + e.record.commit(); + } + + }); }; Ext.extend(MODx.grid.FCSetTabs,MODx.grid.LocalGrid,{ getMenu: function(g,ri) { @@ -460,6 +473,14 @@ MODx.grid.FCSetTVs = function(config) { }); MODx.grid.FCSetTVs.superclass.constructor.call(this,config); this.propRecord = Ext.data.Record.create(config.fields); + this.on('afteredit', function(e) { + if (e.field === 'label') { + const value = MODx.util.stripAndEncode.run(e.value); + e.record.set('label', value); + e.record.commit(); + } + + }); }; Ext.extend(MODx.grid.FCSetTVs,MODx.grid.LocalGrid,{ }); diff --git a/manager/assets/modext/widgets/windows.js b/manager/assets/modext/widgets/windows.js index f49a7511fbc..d1054f9a6a3 100644 --- a/manager/assets/modext/widgets/windows.js +++ b/manager/assets/modext/widgets/windows.js @@ -248,6 +248,13 @@ MODx.window.CreateCategory = function(config) { ,id: 'modx-'+this.ident+'-category' ,xtype: 'textfield' ,anchor: '100%' + ,allowBlank: false + ,blankText: _('category_err_ns_name') + ,listeners: { + change: { + fn: MODx.util.stripAndEncode.onChange + } + } },{ fieldLabel: _('parent') ,name: 'parent' @@ -296,6 +303,13 @@ MODx.window.RenameCategory = function(config) { ,width: 150 ,value: config.record.category ,anchor: '100%' + ,allowBlank: false + ,blankText: _('category_err_ns_name') + ,listeners: { + change: { + fn: MODx.util.stripAndEncode.onChange + } + } },{ fieldLabel: _('rank') ,name: 'rank' @@ -398,6 +412,13 @@ MODx.window.QuickCreateChunk = function(config) { ,name: 'name' ,fieldLabel: _('name') ,anchor: '100%' + ,allowBlank: false + ,blankText: _('chunk_err_ns_name') + ,listeners: { + change: { + fn: MODx.util.stripAndEncode.onChange + } + } },{ xtype: 'modx-combo-category' ,name: 'category' @@ -408,6 +429,11 @@ MODx.window.QuickCreateChunk = function(config) { ,name: 'description' ,fieldLabel: _('description') ,anchor: '100%' + ,listeners: { + change: { + fn: MODx.util.stripAndEncode.onChange + } + } },{ xtype: 'textarea' ,name: 'snippet' @@ -479,6 +505,13 @@ MODx.window.QuickCreateTemplate = function(config) { ,name: 'templatename' ,fieldLabel: _('name') ,anchor: '100%' + ,allowBlank: false + ,blankText: _('template_err_ns_name') + ,listeners: { + change: { + fn: MODx.util.stripAndEncode.onChange + } + } },{ xtype: 'modx-combo-category' ,name: 'category' @@ -489,6 +522,11 @@ MODx.window.QuickCreateTemplate = function(config) { ,name: 'description' ,fieldLabel: _('description') ,anchor: '100%' + ,listeners: { + change: { + fn: MODx.util.stripAndEncode.onChange + } + } },{ xtype: 'textarea' ,name: 'content' @@ -561,6 +599,13 @@ MODx.window.QuickCreateSnippet = function(config) { ,name: 'name' ,fieldLabel: _('name') ,anchor: '100%' + ,allowBlank: false + ,blankText: _('snippet_err_ns_name') + ,listeners: { + change: { + fn: MODx.util.stripAndEncode.onChange + } + } },{ xtype: 'modx-combo-category' ,name: 'category' @@ -571,6 +616,11 @@ MODx.window.QuickCreateSnippet = function(config) { ,name: 'description' ,fieldLabel: _('description') ,anchor: '100%' + ,listeners: { + change: { + fn: MODx.util.stripAndEncode.onChange + } + } },{ xtype: 'textarea' ,name: 'snippet' @@ -644,6 +694,13 @@ MODx.window.QuickCreatePlugin = function(config) { ,name: 'name' ,fieldLabel: _('name') ,anchor: '100%' + ,allowBlank: false + ,blankText: _('plugin_err_ns_name') + ,listeners: { + change: { + fn: MODx.util.stripAndEncode.onChange + } + } },{ xtype: 'modx-combo-category' ,name: 'category' @@ -655,6 +712,11 @@ MODx.window.QuickCreatePlugin = function(config) { ,fieldLabel: _('description') ,anchor: '100%' ,rows: 2 + ,listeners: { + change: { + fn: MODx.util.stripAndEncode.onChange + } + } },{ xtype: 'textarea' ,name: 'plugincode' @@ -740,12 +802,24 @@ MODx.window.QuickCreateTV = function(config) { ,name: 'name' ,fieldLabel: _('name') ,anchor: '100%' + ,allowBlank: false + ,blankText: _('tv_err_ns_name') + ,listeners: { + change: { + fn: MODx.util.stripAndEncode.onChange + } + } },{ xtype: 'textfield' ,name: 'caption' ,id: 'modx-'+this.ident+'-caption' ,fieldLabel: _('caption') ,anchor: '100%' + ,listeners: { + change: { + fn: MODx.util.stripAndEncode.onChange + } + } },{ xtype: 'label' ,forId: 'modx-'+this.ident+'-caption' @@ -761,6 +835,11 @@ MODx.window.QuickCreateTV = function(config) { ,name: 'description' ,fieldLabel: _('description') ,anchor: '100%' + ,listeners: { + change: { + fn: MODx.util.stripAndEncode.onChange + } + } }] },{ columnWidth: .4 From 2730a34933985c55057070db4b0580df08e1d361 Mon Sep 17 00:00:00 2001 From: Jim Graham Date: Thu, 20 Jan 2022 00:33:44 -0500 Subject: [PATCH 4/9] 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). --- .../data/transport.core.system_settings.php | 36 ++++++++ core/lexicon/en/setting.inc.php | 12 +++ .../Revolution/Processors/Element/Create.php | 21 ++++- .../Revolution/Processors/Element/Update.php | 21 ++++- .../Processors/Security/Forms/Set/Update.php | 13 ++- core/src/Revolution/modTemplateVar.php | 68 ++++++++------- core/src/Revolution/modX.php | 85 +++++++++++++++++++ manager/assets/modext/util/utilities.js | 26 ++++-- .../modext/widgets/element/modx.panel.tv.js | 31 ++++++- .../modext/widgets/fc/modx.panel.fcset.js | 9 +- manager/assets/modext/widgets/windows.js | 29 ++++++- 11 files changed, 297 insertions(+), 54 deletions(-) diff --git a/_build/data/transport.core.system_settings.php b/_build/data/transport.core.system_settings.php index cad437d613a..a9743620f87 100644 --- 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 index b877ea9d7c1..4d85eb44b81 100644 --- a/core/lexicon/en/setting.inc.php +++ b/core/lexicon/en/setting.inc.php @@ -240,6 +240,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 index f188a407420..457f01607e3 100644 --- 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 index 075e0923280..f570e487721 100644 --- 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 index 88f67b6aabb..bfdbeaa2372 100644 --- a/core/src/Revolution/Processors/Security/Forms/Set/Update.php +++ b/core/src/Revolution/Processors/Security/Forms/Set/Update.php @@ -1,4 +1,5 @@ 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 index 8149f39ef5f..e0a87979549 100644 --- 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'] = $dbtags['+dbname'] = $this->xpdo->getOption('dbname'); $dbtags['PREFIX'] = $dbtags['+table_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 */ @@ -1005,9 +1011,9 @@ public function parseBinding($binding_string) $properties = []; if (strtoupper($match[1]) != 'SELECT' && 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) { diff --git a/core/src/Revolution/modX.php b/core/src/Revolution/modX.php index 19859b300ec..c76267cdbd4 100644 --- a/core/src/Revolution/modX.php +++ b/core/src/Revolution/modX.php @@ -2981,4 +2981,89 @@ public function _postProcess() { } $this->invokeEvent('OnWebPageComplete'); } + + /** + * Removes unwanted tags and/or tag attributes from an HTML string + * + * @param string $htmlSource The html string to clean + * @param string|array $allowedTags An array or comma-separated list of tag names to allow + * @param string|array $allowedAttr An array or comma-separated list of tag attribute names to allow + */ + public function stripHTML(string $htmlSource, $allowedTags = '', $allowedAttr = ''): string + { + libxml_use_internal_errors(true); + + $allowedTags = is_string($allowedTags) ? trim($allowedTags) : $allowedTags ; + + if (empty($allowedTags)) { + return strip_tags($htmlSource); + } + + if (!is_array($allowedTags)) { + $allowedTags = preg_replace('/[\s<>]+/', '', $allowedTags); + $allowedTags = explode(',', $allowedTags); + } else { + $allowedTags = array_map(function ($tag) { + return preg_replace('/[\s<>]+/', '', $tag); + }, $allowedTags); + } + + if (!empty($allowedAttr)) { + if (!is_array($allowedAttr)) { + $allowedAttr = explode(',', $allowedAttr); + } + $allowedAttr = array_map('trim', $allowedAttr); + } else { + $allowedAttr = []; + } + + $dom = new \DOMDocument(); + + // Need a placeholder wrapping tag, as loadHTML will automatically wrap strings with no root tag with a

tag (do not want that) + $dom->loadHTML(mb_convert_encoding('' . $htmlSource . '', 'HTML-ENTITIES', 'UTF-8'), LIBXML_HTML_NOIMPLIED | LIBXML_HTML_NODEFDTD); + + $xpath = new \DOMXPath($dom); + + foreach ($xpath->query("//*") as $node) { + if (in_array($node->nodeName, $allowedTags)) { + if ($node->attributes->length > 0) { + if (empty($allowedAttr)) { + for ($i = 0; $i < $node->attributes->length; $i++) { + $node->removeAttribute($node->attributes->item(0)->nodeName); + } + } else { + $nodeAttrRemove = []; + for ($i = 0; $i < $node->attributes->length; $i++) { + $name = $node->attributes->item($i)->nodeName; + if (in_array($name, $allowedAttr)) { + $testVal = preg_replace('/[^a-zA-Z:]+/', '', $node->attributes->item($i)->nodeValue); + if (stripos($testVal, 'javascript:') !== false) { + $node->attributes->item($i)->nodeValue = '#js-not-allowed#'; + } + } else { + $nodeAttrRemove[] = $name; + } + } + foreach ($nodeAttrRemove as $attr) { + $node->removeAttribute($attr); + } + } + } + } else { + $parent = $node->parentNode; + while ($node->hasChildNodes()) { + $parent->insertBefore($node->lastChild, $node->nextSibling); + } + $parent->removeChild($node); + } + } + + $output = $dom->saveHTML(); + + // The loop above should already have dropped this placeholder tag, but just in case it did not... + if (strpos($output, '') !== false) { + $output = str_replace(['', ''], '', $output); + } + return $output; + } } diff --git a/manager/assets/modext/util/utilities.js b/manager/assets/modext/util/utilities.js index 86cd021cb49..80daeccb0db 100644 --- a/manager/assets/modext/util/utilities.js +++ b/manager/assets/modext/util/utilities.js @@ -139,27 +139,41 @@ Ext.reg('staticboolean',MODx.StaticBoolean); // replaces javascript invocation in a href attribute and masks html event attributes // in an input string - assuming the result is safe to be displayed by a browser MODx.util.safeHtml = function (input, allowedTags, allowedAttributes) { - var strip = function(input, allowedTags, allowedAttributes) { + + const strip = function(input, allowedTags, allowedAttributes) { return input.replace(tags, function ($0, $1) { return allowedTags.indexOf('<' + $1.toLowerCase() + '>') > -1 ? $0 : ''; }).replace(attributes, function ($0, $1) { return allowedAttributes.indexOf($1.toLowerCase() + ',') > -1 ? $0 : ''; }); }; + + // Convert comma-separated list to angle-bracketed list + if (allowedTags && allowedTags.indexOf('<') === -1) { + allowedTags = allowedTags.replace(/[\s]+/g, ''); + allowedTags = `<${allowedTags.replace(/[,]+/g, '><')}>`; + } + + // Ensure allowedTags arg is a string containing only tags in lowercase () allowedTags = (((allowedTags || '
') + '') .toLowerCase() .match(/<[a-z][a-z0-9]*>/g) || []) - .join(''); // making sure the allowedTags arg is a string containing only tags in lowercase () + .join(''); + + // Ensure allowedAttributes arg is a comma-separated string containing only attributes in lowercase (a,b,c) allowedAttributes = (((allowedAttributes || 'href,class') + '') .toLowerCase() .match(/[a-z\-,]*/g) || []) - .join('').concat(','); // making sure the allowedAttributes arg is a comma separated string containing only attributes in lowercase (a,b,c) - var tags = /<\/?([a-z][a-z0-9]*)\b[^>]*>/gi, + .join('') + .concat(','); + + const tags = /<\/?([a-z][a-z0-9]*)\b[^>]*>/gi, attributes = /([a-z][a-z0-9]*)\s*=\s*".*?"/gi, eventAttributes = /on([a-z][a-z0-9]*\s*=)/gi, commentsAndPhpTags = /|<\?(?:php)?[\s\S]*?\?>/gi, - hrefJavascript = /href(\s*?=\s*?(["'])javascript:.*?\2|\s*?=\s*?javascript:.*?(?![^> ]))/gi, - length; + hrefJavascript = /href(\s*?=\s*?(["'])javascript:.*?\2|\s*?=\s*?javascript:.*?(?![^> ]))/gi + ; + let length; input = input.replace(commentsAndPhpTags, '').replace(hrefJavascript, 'href="javascript:void(0)"'); do { length = input.length; diff --git a/manager/assets/modext/widgets/element/modx.panel.tv.js b/manager/assets/modext/widgets/element/modx.panel.tv.js index fcc4a097b7e..00885ae4b25 100644 --- a/manager/assets/modext/widgets/element/modx.panel.tv.js +++ b/manager/assets/modext/widgets/element/modx.panel.tv.js @@ -101,7 +101,12 @@ MODx.panel.TV = function(config) { ,scope: this }, change: { - fn: MODx.util.stripAndEncode.onChange + fn: function(cmp) { + const value = cmp.getValue().trim(); + if (value.length > 0) { + cmp.setValue(Ext.util.Format.stripTags(value)); + } + } }, blur: { fn: function(cmp) { @@ -143,7 +148,7 @@ MODx.panel.TV = function(config) { ,tabIndex: 2 ,listeners: { afterrender: {scope:this,fn:function(f,e) { - MODx.setStaticElementPath('tv'); + MODx.setStaticElementPath('tv'); }} ,select: {scope:this,fn:function(f,e) { MODx.setStaticElementPath('tv'); @@ -187,7 +192,16 @@ MODx.panel.TV = function(config) { ,value: config.record.caption ,listeners: { change: { - fn: MODx.util.stripAndEncode.onChange + fn: function(cmp) { + const value = cmp.getValue().trim(); + if (value.length > 0) { + cmp.setValue(MODx.util.safeHtml( + value, + MODx.config.elements_caption_allowedtags, + MODx.config.elements_caption_allowedattr + )); + } + } } } },{ @@ -254,7 +268,16 @@ MODx.panel.TV = function(config) { ,value: config.record.description || '' ,listeners: { change: { - fn: MODx.util.stripAndEncode.onChange + fn: function(cmp) { + const value = cmp.getValue().trim(); + if (value.length > 0) { + cmp.setValue(MODx.util.safeHtml( + value, + MODx.config.elements_description_allowedtags, + MODx.config.elements_description_allowedattr + )); + } + } } } },{ diff --git a/manager/assets/modext/widgets/fc/modx.panel.fcset.js b/manager/assets/modext/widgets/fc/modx.panel.fcset.js index 771fd9a1269..f15bb8be46b 100644 --- a/manager/assets/modext/widgets/fc/modx.panel.fcset.js +++ b/manager/assets/modext/widgets/fc/modx.panel.fcset.js @@ -475,7 +475,14 @@ MODx.grid.FCSetTVs = function(config) { this.propRecord = Ext.data.Record.create(config.fields); this.on('afteredit', function(e) { if (e.field === 'label') { - const value = MODx.util.stripAndEncode.run(e.value); + let value = e.value.trim(); + if (value.length > 0) { + value = MODx.util.safeHtml( + value, + MODx.config.elements_caption_allowedtags, + MODx.config.elements_caption_allowedattr + ) + } e.record.set('label', value); e.record.commit(); } diff --git a/manager/assets/modext/widgets/windows.js b/manager/assets/modext/widgets/windows.js index d1054f9a6a3..77f80598d95 100644 --- a/manager/assets/modext/widgets/windows.js +++ b/manager/assets/modext/widgets/windows.js @@ -806,7 +806,12 @@ MODx.window.QuickCreateTV = function(config) { ,blankText: _('tv_err_ns_name') ,listeners: { change: { - fn: MODx.util.stripAndEncode.onChange + fn: function(cmp) { + const value = cmp.getValue().trim(); + if (value.length > 0) { + cmp.setValue(Ext.util.Format.stripTags(value)); + } + } } } },{ @@ -817,7 +822,16 @@ MODx.window.QuickCreateTV = function(config) { ,anchor: '100%' ,listeners: { change: { - fn: MODx.util.stripAndEncode.onChange + fn: function(cmp) { + const value = cmp.getValue().trim(); + if (value.length > 0) { + cmp.setValue(MODx.util.safeHtml( + value, + MODx.config.elements_caption_allowedtags, + MODx.config.elements_caption_allowedattr + )); + } + } } } },{ @@ -837,7 +851,16 @@ MODx.window.QuickCreateTV = function(config) { ,anchor: '100%' ,listeners: { change: { - fn: MODx.util.stripAndEncode.onChange + fn: function(cmp) { + const value = cmp.getValue().trim(); + if (value.length > 0) { + cmp.setValue(MODx.util.safeHtml( + value, + MODx.config.elements_description_allowedtags, + MODx.config.elements_description_allowedattr + )); + } + } } } }] From e4ccd3daf7164c82f17a687351d91f8f5cd03452 Mon Sep 17 00:00:00 2001 From: Jim Graham Date: Sat, 2 Apr 2022 00:35:16 -0400 Subject: [PATCH 5/9] Fix incorrect object reference --- core/src/Revolution/modTemplateVar.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/core/src/Revolution/modTemplateVar.php b/core/src/Revolution/modTemplateVar.php index e0a87979549..b2b268baa63 100644 --- a/core/src/Revolution/modTemplateVar.php +++ b/core/src/Revolution/modTemplateVar.php @@ -406,10 +406,10 @@ public function renderInput($resource = null, $options = []) $this->set('default_text', $this->processBindings($this->get('default_text'), $resourceId)); /* remove disallowed tags and attributes from description */ - $this->set('description', $this->modx->stripHtml( + $this->set('description', $this->xpdo->stripHtml( $this->get('description'), - $this->modx->getOption('elements_description_allowedtags'), - $this->modx->getOption('elements_description_allowedattr') + $this->xpdo->getOption('elements_description_allowedtags'), + $this->xpdo->getOption('elements_description_allowedattr') )); $params = []; From d291929de702561356444011f05ed350f507e572 Mon Sep 17 00:00:00 2001 From: Jim Graham Date: Fri, 6 May 2022 01:15:55 -0400 Subject: [PATCH 6/9] Reincorporate changes lost in rebase --- .../assets/modext/widgets/element/modx.panel.chunk.js | 9 +++++++++ .../assets/modext/widgets/element/modx.panel.plugin.js | 9 +++++++++ .../assets/modext/widgets/element/modx.panel.snippet.js | 9 +++++++++ .../assets/modext/widgets/element/modx.panel.template.js | 9 +++++++++ 4 files changed, 36 insertions(+) diff --git a/manager/assets/modext/widgets/element/modx.panel.chunk.js b/manager/assets/modext/widgets/element/modx.panel.chunk.js index ee9afc4bd18..4b97c70e359 100644 --- a/manager/assets/modext/widgets/element/modx.panel.chunk.js +++ b/manager/assets/modext/widgets/element/modx.panel.chunk.js @@ -82,6 +82,7 @@ MODx.panel.Chunk = function(config) { ,maxLength: 50 ,enableKeyEvents: true ,allowBlank: false + ,blankText: _('chunk_err_ns_name') ,value: config.record.name ,tabIndex: 1 ,listeners: { @@ -95,6 +96,9 @@ MODx.panel.Chunk = function(config) { } ,scope: this } + ,change: { + fn: MODx.util.stripAndEncode.onChange + } } },{ xtype: MODx.expandHelp ? 'label' : 'hidden' @@ -173,6 +177,11 @@ MODx.panel.Chunk = function(config) { ,maxLength: 255 ,tabIndex: 5 ,value: config.record.description || '' + ,listeners: { + change: { + fn: MODx.util.stripAndEncode.onChange + } + } },{ xtype: MODx.expandHelp ? 'label' : 'hidden' ,forId: 'modx-chunk-description' diff --git a/manager/assets/modext/widgets/element/modx.panel.plugin.js b/manager/assets/modext/widgets/element/modx.panel.plugin.js index e63ce2d9a50..2af200d55e8 100644 --- a/manager/assets/modext/widgets/element/modx.panel.plugin.js +++ b/manager/assets/modext/widgets/element/modx.panel.plugin.js @@ -82,6 +82,7 @@ MODx.panel.Plugin = function(config) { ,maxLength: 50 ,enableKeyEvents: true ,allowBlank: false + ,blankText: _('plugin_err_ns_name') ,value: config.record.name ,tabIndex: 1 ,listeners: { @@ -92,6 +93,9 @@ MODx.panel.Plugin = function(config) { } ,scope: this } + ,change: { + fn: MODx.util.stripAndEncode.onChange + } } },{ xtype: MODx.expandHelp ? 'label' : 'hidden' @@ -160,6 +164,11 @@ MODx.panel.Plugin = function(config) { ,maxLength: 255 ,tabIndex: 3 ,value: config.record.description || '' + ,listeners: { + change: { + fn: MODx.util.stripAndEncode.onChange + } + } },{ xtype: MODx.expandHelp ? 'label' : 'hidden' ,forId: 'modx-plugin-description' diff --git a/manager/assets/modext/widgets/element/modx.panel.snippet.js b/manager/assets/modext/widgets/element/modx.panel.snippet.js index fb5ba2ca7a8..59903938b32 100644 --- a/manager/assets/modext/widgets/element/modx.panel.snippet.js +++ b/manager/assets/modext/widgets/element/modx.panel.snippet.js @@ -85,6 +85,7 @@ MODx.panel.Snippet = function(config) { ,maxLength: 50 ,enableKeyEvents: true ,allowBlank: false + ,blankText: _('snippet_err_ns_name') ,value: config.record.name ,tabIndex: 1 ,listeners: { @@ -98,6 +99,9 @@ MODx.panel.Snippet = function(config) { } ,scope: this } + ,change: { + fn: MODx.util.stripAndEncode.onChange + } } },{ xtype: MODx.expandHelp ? 'label' : 'hidden' @@ -176,6 +180,11 @@ MODx.panel.Snippet = function(config) { ,maxLength: 255 ,tabIndex: 3 ,value: config.record.description || '' + ,listeners: { + change: { + fn: MODx.util.stripAndEncode.onChange + } + } },{ xtype: MODx.expandHelp ? 'label' : 'hidden' ,forId: 'modx-snippet-description' diff --git a/manager/assets/modext/widgets/element/modx.panel.template.js b/manager/assets/modext/widgets/element/modx.panel.template.js index 18d5e0d1e49..527effb2dad 100644 --- a/manager/assets/modext/widgets/element/modx.panel.template.js +++ b/manager/assets/modext/widgets/element/modx.panel.template.js @@ -83,6 +83,7 @@ MODx.panel.Template = function(config) { ,maxLength: 50 ,enableKeyEvents: true ,allowBlank: false + ,blankText: _('template_err_ns_name') ,value: config.record.templatename ,tabIndex: 1 ,listeners: { @@ -93,6 +94,9 @@ MODx.panel.Template = function(config) { } ,scope: this } + ,change: { + fn: MODx.util.stripAndEncode.onChange + } } },{ xtype: MODx.expandHelp ? 'label' : 'hidden' @@ -161,6 +165,11 @@ MODx.panel.Template = function(config) { ,maxLength: 255 ,tabIndex: 5 ,value: config.record.description || '' + ,listeners: { + change: { + fn: MODx.util.stripAndEncode.onChange + } + } },{ xtype: MODx.expandHelp ? 'label' : 'hidden' ,forId: 'modx-template-description' From bcd2f11cce7fdc36900c70b445f516f901afcf08 Mon Sep 17 00:00:00 2001 From: Jim Graham Date: Fri, 6 May 2022 01:51:01 -0400 Subject: [PATCH 7/9] Minor consistency tweaks --- core/lexicon/en/plugin.inc.php | 2 +- core/lexicon/en/snippet.inc.php | 2 +- core/src/Revolution/Processors/Element/Create.php | 6 +++--- core/src/Revolution/Processors/Element/Update.php | 15 ++++++++------- 4 files changed, 13 insertions(+), 12 deletions(-) diff --git a/core/lexicon/en/plugin.inc.php b/core/lexicon/en/plugin.inc.php index a153e8841c9..25d0411a2e7 100644 --- a/core/lexicon/en/plugin.inc.php +++ b/core/lexicon/en/plugin.inc.php @@ -24,7 +24,7 @@ $_lang['plugin_err_duplicate'] = 'An error occurred while trying to duplicate the plugin.'; $_lang['plugin_err_nf'] = 'Plugin not found!'; $_lang['plugin_err_ns'] = 'Plugin not specified.'; -$_lang['plugin_err_ns_name'] = 'Please specify a name for the plugin.'; +$_lang['plugin_err_ns_name'] = 'Please specify a name for this plugin.'; $_lang['plugin_err_remove'] = 'An error occurred while trying to delete the plugin.'; $_lang['plugin_err_save'] = 'An error occurred while saving the plugin.'; $_lang['plugin_event_err_duplicate'] = 'An error occurred while trying to duplicate the plugin events'; diff --git a/core/lexicon/en/snippet.inc.php b/core/lexicon/en/snippet.inc.php index b6f45c5b654..11888d79bc7 100644 --- a/core/lexicon/en/snippet.inc.php +++ b/core/lexicon/en/snippet.inc.php @@ -23,7 +23,7 @@ $_lang['snippet_err_locked'] = 'This snippet is locked for editing.'; $_lang['snippet_err_nf'] = 'Snippet not found!'; $_lang['snippet_err_ns'] = 'Snippet not specified.'; -$_lang['snippet_err_ns_name'] = 'Please specify a name for the snippet.'; +$_lang['snippet_err_ns_name'] = 'Please specify a name for this snippet.'; $_lang['snippet_err_remove'] = 'An error occurred while trying to delete the snippet.'; $_lang['snippet_err_save'] = 'An error occurred while saving the snippet.'; $_lang['snippet_execonsave'] = 'Execute snippet after saving.'; diff --git a/core/src/Revolution/Processors/Element/Create.php b/core/src/Revolution/Processors/Element/Create.php index 457f01607e3..678fd502817 100644 --- a/core/src/Revolution/Processors/Element/Create.php +++ b/core/src/Revolution/Processors/Element/Create.php @@ -86,15 +86,15 @@ public function beforeSave() $this->object->set('description', $description); } - $name = $this->getProperty($this->elementNameField, ''); - /* verify element has a name and that name does not already exist */ + $name = $this->getProperty($this->elementNameField, ''); + if (empty($name)) { $this->addFieldError($this->elementNameField, $this->modx->lexicon($this->objectType . '_err_ns_name')); } else { if ($this->alreadyExists($name)) { - $this->modx->error->addField( + $this->addFieldError( $this->elementNameField, $this->modx->lexicon($this->objectType . '_err_ae', ['name' => $name]) ); diff --git a/core/src/Revolution/Processors/Element/Update.php b/core/src/Revolution/Processors/Element/Update.php index f570e487721..888c14b75fc 100644 --- a/core/src/Revolution/Processors/Element/Update.php +++ b/core/src/Revolution/Processors/Element/Update.php @@ -86,13 +86,14 @@ public function beforeSave() $name = $this->getProperty($this->elementNameField, ''); if (empty($name)) { - $this->addFieldError($nameField, $this->modx->lexicon($this->objectType . '_err_ns_name')); - } else if ($this->alreadyExists($name)) { - /* if changing name, but new one already exists */ - $this->modx->error->addField( - $nameField, - $this->modx->lexicon($this->objectType . '_err_ae', ['name' => $name]) - ); + $this->addFieldError($this->elementNameField, $this->modx->lexicon($this->objectType . '_err_ns_name')); + } else { + if ($this->alreadyExists($name)) { + $this->addFieldError( + $this->elementNameField, + $this->modx->lexicon($this->objectType . '_err_ae', ['name' => $name]) + ); + } } /* category */ From f611c3bc0156b07b9398f9a3d9ed6f026eea5a84 Mon Sep 17 00:00:00 2001 From: Jim Graham Date: Sat, 21 Jan 2023 15:31:38 -0500 Subject: [PATCH 8/9] Update core/lexicon/en/setting.inc.php MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Joshua Lückers --- core/lexicon/en/setting.inc.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/core/lexicon/en/setting.inc.php b/core/lexicon/en/setting.inc.php index 4d85eb44b81..38f84b5de0c 100644 --- a/core/lexicon/en/setting.inc.php +++ b/core/lexicon/en/setting.inc.php @@ -241,16 +241,16 @@ $_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_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_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_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_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.'; From c6812e424c946107895377f17cfbcf4f0b52c0db Mon Sep 17 00:00:00 2001 From: Jim Graham Date: Sat, 21 Jan 2023 16:38:58 -0500 Subject: [PATCH 9/9] Use fully-qualified classname in conditional Change made by request for better maintainability. --- core/src/Revolution/Processors/Element/Create.php | 7 ++++--- core/src/Revolution/Processors/Element/Update.php | 7 ++++--- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/core/src/Revolution/Processors/Element/Create.php b/core/src/Revolution/Processors/Element/Create.php index 678fd502817..63b504eed01 100644 --- a/core/src/Revolution/Processors/Element/Create.php +++ b/core/src/Revolution/Processors/Element/Create.php @@ -15,6 +15,7 @@ use MODX\Revolution\modElement; use MODX\Revolution\Processors\Model\CreateProcessor; use MODX\Revolution\modTemplate; +use MODX\Revolution\modTemplateVar; use MODX\Revolution\Validation\modValidator; /** @@ -61,9 +62,9 @@ public function beforeSave() $locked = (bool)$this->getProperty('locked', false); $this->object->set('locked', $locked); - $elementClassName = array_pop(explode('\\', $this->classKey)); + $isTV = $this->classKey === modTemplateVar::class; - if ($elementClassName === 'modTemplateVar') { + if ($isTV) { if ($caption = trim($this->getProperty('caption', ''))) { $caption = $this->modx->stripHtml( $caption, @@ -75,7 +76,7 @@ public function beforeSave() } if ($description = trim($this->getProperty('description', ''))) { - $description = $elementClassName === 'modTemplateVar' + $description = $isTV ? $this->modx->stripHtml( $description, $this->modx->getOption('elements_description_allowedtags'), diff --git a/core/src/Revolution/Processors/Element/Update.php b/core/src/Revolution/Processors/Element/Update.php index 888c14b75fc..64446e441b0 100644 --- a/core/src/Revolution/Processors/Element/Update.php +++ b/core/src/Revolution/Processors/Element/Update.php @@ -15,6 +15,7 @@ use MODX\Revolution\modElement; use MODX\Revolution\Processors\Model\UpdateProcessor; use MODX\Revolution\modTemplate; +use MODX\Revolution\modTemplateVar; /** * Abstract class for Update Element processors. To be extended for each derivative element type. @@ -56,9 +57,9 @@ public function beforeSave() $this->object->set('locked', (bool)$locked); } - $elementClassName = array_pop(explode('\\', $this->classKey)); + $isTV = $this->classKey === modTemplateVar::class; - if ($elementClassName === 'modTemplateVar') { + if ($isTV) { if ($caption = trim($this->getProperty('caption', ''))) { $caption = $this->modx->stripHtml( $caption, @@ -70,7 +71,7 @@ public function beforeSave() } if ($description = trim($this->getProperty('description', ''))) { - $description = $elementClassName === 'modTemplateVar' + $description = $isTV ? $this->modx->stripHtml( $description, $this->modx->getOption('elements_description_allowedtags'),