Skip to content

Commit 0ad1d00

Browse files
author
Jim Graham
committed
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).
1 parent 3437ac0 commit 0ad1d00

File tree

12 files changed

+299
-56
lines changed

12 files changed

+299
-56
lines changed

Diff for: ‎_build/data/transport.core.system_settings.php

+36
Original file line numberDiff line numberDiff line change
@@ -470,6 +470,42 @@
470470
'area' => 'site',
471471
'editedon' => null,
472472
], '', true, true);
473+
$settings['elements_caption_allowedattr'] = $xpdo->newObject(modSystemSetting::class);
474+
$settings['elements_caption_allowedattr']->fromArray([
475+
'key' => 'elements_caption_allowedattr',
476+
'value' => 'href',
477+
'xtype' => 'textfield',
478+
'namespace' => 'core',
479+
'area' => 'manager',
480+
'editedon' => null,
481+
], '', true, true);
482+
$settings['elements_caption_allowedtags'] = $xpdo->newObject(modSystemSetting::class);
483+
$settings['elements_caption_allowedtags']->fromArray([
484+
'key' => 'elements_caption_allowedtags',
485+
'value' => 'a',
486+
'xtype' => 'textfield',
487+
'namespace' => 'core',
488+
'area' => 'manager',
489+
'editedon' => null,
490+
], '', true, true);
491+
$settings['elements_description_allowedattr'] = $xpdo->newObject(modSystemSetting::class);
492+
$settings['elements_description_allowedattr']->fromArray([
493+
'key' => 'elements_description_allowedattr',
494+
'value' => 'href,src,class',
495+
'xtype' => 'textfield',
496+
'namespace' => 'core',
497+
'area' => 'manager',
498+
'editedon' => null,
499+
], '', true, true);
500+
$settings['elements_description_allowedtags'] = $xpdo->newObject(modSystemSetting::class);
501+
$settings['elements_description_allowedtags']->fromArray([
502+
'key' => 'elements_description_allowedtags',
503+
'value' => 'div,p,ul,ol,li,img,span,br,strong,b,em,i,a',
504+
'xtype' => 'textfield',
505+
'namespace' => 'core',
506+
'area' => 'manager',
507+
'editedon' => null,
508+
], '', true, true);
473509
$settings['emailsender'] = $xpdo->newObject(modSystemSetting::class);
474510
$settings['emailsender']->fromArray([
475511
'key' => 'emailsender',

Diff for: ‎core/lexicon/en/setting.inc.php

+12
Original file line numberDiff line numberDiff line change
@@ -240,6 +240,18 @@
240240
$_lang['setting_default_per_page'] = 'Default Per Page';
241241
$_lang['setting_default_per_page_desc'] = 'The default number of results to show in grids throughout the manager.';
242242

243+
$_lang['setting_elements_caption_allowedattr'] = 'Element Captions: Allowed Attributes';
244+
$_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).';
245+
246+
$_lang['setting_elements_caption_allowedtags'] = 'Element Captions: Allowed Tags';
247+
$_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).';
248+
249+
$_lang['setting_elements_description_allowedattr'] = 'Element Descriptions: Allowed Attributes';
250+
$_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.';
251+
252+
$_lang['setting_elements_description_allowedtags'] = 'Element Descriptions: Allowed Tags';
253+
$_lang['setting_elements_description_allowedtags_desc'] = 'When adding an element description, the html tag(s) provided in this comma-separated list will be preserved.';
254+
243255
$_lang['setting_emailsender'] = 'Registration Email From Address';
244256
$_lang['setting_emailsender_desc'] = 'Here you can specify the email address used when sending Users their usernames and passwords.';
245257
$_lang['setting_emailsender_err'] = 'Please state the administration email address.';

Diff for: ‎core/src/Revolution/Processors/Element/Create.php

+17-4
Original file line numberDiff line numberDiff line change
@@ -64,13 +64,26 @@ public function beforeSave()
6464
$elementClassName = array_pop(explode('\\', $this->classKey));
6565

6666
if ($elementClassName === 'modTemplateVar') {
67-
if ($caption = $this->getProperty('caption', '')) {
68-
$this->object->set('caption', strip_tags($caption));
67+
if ($caption = trim($this->getProperty('caption', ''))) {
68+
$caption = $this->modx->stripHtml(
69+
$caption,
70+
$this->modx->getOption('elements_caption_allowedtags'),
71+
$this->modx->getOption('elements_caption_allowedattr')
72+
);
73+
$this->object->set('caption', $caption);
6974
}
7075
}
7176

72-
if ($description = $this->getProperty('description', '')) {
73-
$this->object->set('description', strip_tags($description));
77+
if ($description = trim($this->getProperty('description', ''))) {
78+
$description = $elementClassName === 'modTemplateVar'
79+
? $this->modx->stripHtml(
80+
$description,
81+
$this->modx->getOption('elements_description_allowedtags'),
82+
$this->modx->getOption('elements_description_allowedattr')
83+
)
84+
: strip_tags($description)
85+
;
86+
$this->object->set('description', $description);
7487
}
7588

7689
$name = $this->getProperty($this->elementNameField, '');

Diff for: ‎core/src/Revolution/Processors/Element/Update.php

+17-4
Original file line numberDiff line numberDiff line change
@@ -59,13 +59,26 @@ public function beforeSave()
5959
$elementClassName = array_pop(explode('\\', $this->classKey));
6060

6161
if ($elementClassName === 'modTemplateVar') {
62-
if ($caption = $this->getProperty('caption', '')) {
63-
$this->object->set('caption', strip_tags($caption));
62+
if ($caption = trim($this->getProperty('caption', ''))) {
63+
$caption = $this->modx->stripHtml(
64+
$caption,
65+
$this->modx->getOption('elements_caption_allowedtags'),
66+
$this->modx->getOption('elements_caption_allowedattr')
67+
);
68+
$this->object->set('caption', $caption);
6469
}
6570
}
6671

67-
if ($description = $this->getProperty('description', '')) {
68-
$this->object->set('description', strip_tags($description));
72+
if ($description = trim($this->getProperty('description', ''))) {
73+
$description = $elementClassName === 'modTemplateVar'
74+
? $this->modx->stripHtml(
75+
$description,
76+
$this->modx->getOption('elements_description_allowedtags'),
77+
$this->modx->getOption('elements_description_allowedattr')
78+
)
79+
: strip_tags($description)
80+
;
81+
$this->object->set('description', $description);
6982
}
7083

7184
/* verify element has a name and that name does not already exist */

Diff for: ‎core/src/Revolution/Processors/Security/Forms/Set/Update.php

+11-2
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
<?php
2+
23
/*
34
* This file is part of MODX Revolution.
45
*
@@ -121,7 +122,11 @@ public function setFieldRules()
121122
$this->newRules[] = $rule;
122123
}
123124
if (!empty($field['label'])) {
124-
$field['label'] = strip_tags($field['label']);
125+
$field['label'] = $this->modx->stripHtml(
126+
$field['label'],
127+
$this->modx->getOption('elements_caption_allowedtags'),
128+
$this->modx->getOption('elements_caption_allowedattr')
129+
);
125130
$rule = $this->modx->newObject(modActionDom::class);
126131
$rule->set('set', $this->object->get('id'));
127132
$rule->set('action', $this->object->get('action'));
@@ -287,7 +292,11 @@ public function setTVRules()
287292
$this->newRules[] = $rule;
288293
}
289294
if (!empty($tvData['label'])) {
290-
$tvData['label'] = strip_tags($tvData['label']);
295+
$tvData['label'] = $this->modx->stripHtml(
296+
$tvData['label'],
297+
$this->modx->getOption('elements_caption_allowedtags'),
298+
$this->modx->getOption('elements_caption_allowedattr')
299+
);
291300
$rule = $this->modx->newObject(modActionDom::class);
292301
$rule->set('set', $this->object->get('id'));
293302
$rule->set('action', $this->object->get('action'));

Diff for: ‎core/src/Revolution/modTemplateVar.php

+39-31
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ class modTemplateVar extends modElement
7575
*
7676
* {@inheritdoc}
7777
*/
78-
function __construct(& $xpdo)
78+
public function __construct(&$xpdo)
7979
{
8080
parent:: __construct($xpdo);
8181
$this->setToken('*');
@@ -161,7 +161,7 @@ public function process($properties = null, $content = null)
161161
/* copy the content source to the output buffer */
162162
$this->_output = $this->_content;
163163

164-
if (is_string($this->_output) && !empty ($this->_output)) {
164+
if (is_string($this->_output) && !empty($this->_output)) {
165165
/* turn the processed properties into placeholders */
166166
$scope = $this->xpdo->toPlaceholders($this->_properties, '', '.', true);
167167

@@ -217,10 +217,10 @@ public function getValue($resourceId = 0)
217217
$value = null;
218218
$resourceId = intval($resourceId);
219219
if ($resourceId) {
220-
if (is_object($this->xpdo->resource) && $resourceId === (integer)$this->xpdo->resourceIdentifier && is_array($this->xpdo->resource->get($this->get('name')))) {
220+
if (is_object($this->xpdo->resource) && $resourceId === (int)$this->xpdo->resourceIdentifier && is_array($this->xpdo->resource->get($this->get('name')))) {
221221
$valueArray = $this->xpdo->resource->get($this->get('name'));
222222
$value = $valueArray[1];
223-
} elseif ($resourceId === (integer)$this->get('resourceId') && array_key_exists('value', $this->_fields)) {
223+
} elseif ($resourceId === (int)$this->get('resourceId') && array_key_exists('value', $this->_fields)) {
224224
$value = $this->get('value');
225225
} else {
226226
$resource = $this->xpdo->getObject(modTemplateVarResource::class, [
@@ -269,8 +269,7 @@ public function setValue($resourceId = 0, $value = null)
269269
$templateVarResource->set('value', $value);
270270
}
271271
$this->addMany($templateVarResource);
272-
} elseif (!$templateVarResource->isNew()
273-
&& ($value === null || $value === $this->get('default_text'))) {
272+
} elseif (!$templateVarResource->isNew() && ($value === null || $value === $this->get('default_text'))) {
274273
$templateVarResource->remove();
275274
}
276275
}
@@ -325,8 +324,10 @@ public function prepareOutput($value, $resourceId = 0)
325324
$mTypes = $this->xpdo->getOption('manipulatable_url_tv_output_types', null, 'image,file');
326325
$mTypes = explode(',', $mTypes);
327326
if (!empty($value) && in_array($this->get('type'), $mTypes)) {
328-
$context = !empty($resourceId) ? $this->xpdo->getObject(modResource::class,
329-
$resourceId)->get('context_key') : $this->xpdo->context->get('key');
327+
$context = !empty($resourceId)
328+
? $this->xpdo->getObject(modResource::class, $resourceId)->get('context_key')
329+
: $this->xpdo->context->get('key')
330+
;
330331
$sourceCache = $this->getSourceCache($context);
331332
$classKey = $sourceCache['class_key'];
332333
if (!empty($sourceCache) && !empty($classKey)) {
@@ -336,8 +337,7 @@ public function prepareOutput($value, $resourceId = 0)
336337
if ($source) {
337338
$source->fromArray($sourceCache, '', true, true);
338339
$source->initialize();
339-
$isAbsolute = strpos($value, 'http://') === 0 || strpos($value,
340-
'https://') === 0 || strpos($value, 'ftp://') === 0;
340+
$isAbsolute = strpos($value, 'http://') === 0 || strpos($value, 'https://') === 0 || strpos($value, 'ftp://') === 0;
341341
if (!$isAbsolute) {
342342
$value = $source->prepareOutputUrl($value);
343343
}
@@ -380,8 +380,11 @@ public function renderInput($resource = null, $options = [])
380380
}
381381
if (!isset($this->xpdo->smarty)) {
382382
$this->xpdo->getService('smarty', modSmarty::class, '', [
383-
'template_dir' => $this->xpdo->getOption('manager_path') . 'templates/' . $this->xpdo->getOption('manager_theme',
384-
null, 'default') . '/',
383+
'template_dir' => $this->xpdo->getOption('manager_path') . 'templates/' . $this->xpdo->getOption(
384+
'manager_theme',
385+
null,
386+
'default'
387+
) . '/'
385388
]);
386389
}
387390
$this->xpdo->smarty->assign('style', $style);
@@ -402,8 +405,12 @@ public function renderInput($resource = null, $options = [])
402405
$this->set('processedValue', $value);
403406
$this->set('default_text', $this->processBindings($this->get('default_text'), $resourceId));
404407

405-
/* strip tags from description */
406-
// $this->set('description', strip_tags($this->get('description')));
408+
/* remove disallowed tags and attributes from description */
409+
$this->set('description', $this->modx->stripHtml(
410+
$this->get('description'),
411+
$this->modx->getOption('elements_description_allowedtags'),
412+
$this->modx->getOption('elements_description_allowedattr')
413+
));
407414

408415
$params = [];
409416
if ($paramstring = $this->get('display_params')) {
@@ -627,8 +634,7 @@ public function checkForFormCustomizationRules($value, &$resource)
627634
$c = $this->xpdo->newQuery(modActionDom::class);
628635
$c->innerJoin(modFormCustomizationSet::class, 'FCSet');
629636
$c->innerJoin(modFormCustomizationProfile::class, 'Profile', 'FCSet.profile = Profile.id');
630-
$c->leftJoin(modFormCustomizationProfileUserGroup::class, 'ProfileUserGroup',
631-
'Profile.id = ProfileUserGroup.profile');
637+
$c->leftJoin(modFormCustomizationProfileUserGroup::class, 'ProfileUserGroup', 'Profile.id = ProfileUserGroup.profile');
632638
$c->leftJoin(modFormCustomizationProfile::class, 'UGProfile', 'UGProfile.id = ProfileUserGroup.profile');
633639
$ruleFieldName = $this->xpdo->escape('rule');
634640
$c->where([
@@ -654,8 +660,7 @@ public function checkForFormCustomizationRules($value, &$resource)
654660
], xPDOQuery::SQL_AND, null, 2);
655661
}
656662
if (!empty($this->xpdo->request) && !empty($this->xpdo->request->action)) {
657-
$wildAction = substr($this->xpdo->request->action, 0,
658-
strrpos($this->xpdo->request->action, '/')) . '/*';
663+
$wildAction = substr($this->xpdo->request->action, 0, strrpos($this->xpdo->request->action, '/')) . '/*';
659664
$c->where([
660665
'modActionDom.action:IN' => [$this->xpdo->request->action, $wildAction],
661666
]);
@@ -898,7 +903,7 @@ public function processBindings($value = '', $resourceId = 0, $preProcess = true
898903
case 'DOCUMENT': /* retrieve a document and process it's content */
899904
if ($preProcess) {
900905
$query = $this->xpdo->newQuery(modResource::class, [
901-
'id' => (integer)$param,
906+
'id' => (int)$param,
902907
'deleted' => false,
903908
]);
904909
$query->select('content');
@@ -919,8 +924,10 @@ public function processBindings($value = '', $resourceId = 0, $preProcess = true
919924
$dbtags['DBASE'] = $this->xpdo->getOption('dbname');
920925
$dbtags['PREFIX'] = $this->xpdo->getOption('table_prefix');
921926
foreach ($dbtags as $key => $pValue) {
922-
if (!is_scalar($pValue)) continue;
923-
$param = str_replace('[[+'.$key.']]', (string)$pValue, $param);
927+
if (!is_scalar($pValue)) {
928+
continue;
929+
}
930+
$param = str_replace('[[+' . $key . ']]', (string)$pValue, $param);
924931
}
925932
$stmt = $this->xpdo->query('SELECT ' . $param);
926933
if ($stmt && $stmt instanceof PDOStatement) {
@@ -975,7 +982,6 @@ public function processBindings($value = '', $resourceId = 0, $preProcess = true
975982
default:
976983
$output = $value;
977984
break;
978-
979985
}
980986

981987
/* support for nested bindings */
@@ -1005,11 +1011,11 @@ public function parseBinding($binding_string)
10051011
$regexp2 = '/(\S+)\s+(.+)/is'; /* Split binding on second whitespace to get properties */
10061012

10071013
$properties = [];
1008-
if (preg_match($regexp2, $match[2] , $match2)) {
1014+
if (preg_match($regexp2, $match[2], $match2)) {
10091015
if (isset($match2[2])) {
1010-
$props = json_decode($match2[2],true);
1016+
$props = json_decode($match2[2], true);
10111017
$valid = json_last_error() === JSON_ERROR_NONE;
1012-
if ($valid && is_array($props)){
1018+
if ($valid && is_array($props)) {
10131019
$properties = $props;
10141020
$match[2] = $match2[1];
10151021
} else {
@@ -1041,8 +1047,10 @@ public function processInheritBinding($default = '', $resourceId = null)
10411047
$output = $default; /* Default to param value if no content from parents */
10421048
$resource = null;
10431049
$resourceColumns = $this->xpdo->getSelectColumns(modResource::class, '', '', ['id', 'parent']);
1044-
$resourceQuery = new xPDOCriteria($this->xpdo,
1045-
"SELECT {$resourceColumns} FROM {$this->xpdo->getTableName(modResource::class)} WHERE id = ?");
1050+
$resourceQuery = new xPDOCriteria(
1051+
$this->xpdo,
1052+
"SELECT {$resourceColumns} FROM {$this->xpdo->getTableName(modResource::class)} WHERE id = ?"
1053+
);
10461054
if (!empty($resourceId) && (!($this->xpdo->resource instanceof modResource) || $this->xpdo->resource->get('id') != $resourceId)) {
10471055
if ($resourceQuery->stmt && $resourceQuery->stmt->execute([$resourceId])) {
10481056
$result = $resourceQuery->stmt->fetchAll(PDO::FETCH_ASSOC);
@@ -1115,11 +1123,11 @@ public function findPolicy($context = '')
11151123
$policy = [];
11161124
$context = !empty($context) ? $context : $this->xpdo->context->get('key');
11171125
if ($context === $this->xpdo->context->get('key')) {
1118-
$catEnabled = (boolean)$this->xpdo->getOption('access_category_enabled', null, true);
1119-
$rgEnabled = (boolean)$this->xpdo->getOption('access_resource_group_enabled', null, true);
1126+
$catEnabled = (bool)$this->xpdo->getOption('access_category_enabled', null, true);
1127+
$rgEnabled = (bool)$this->xpdo->getOption('access_resource_group_enabled', null, true);
11201128
} elseif ($this->xpdo->getContext($context)) {
1121-
$catEnabled = (boolean)$this->xpdo->contexts[$context]->getOption('access_category_enabled', true);
1122-
$rgEnabled = (boolean)$this->xpdo->contexts[$context]->getOption('access_resource_group_enabled', true);
1129+
$catEnabled = (bool)$this->xpdo->contexts[$context]->getOption('access_category_enabled', true);
1130+
$rgEnabled = (bool)$this->xpdo->contexts[$context]->getOption('access_resource_group_enabled', true);
11231131
}
11241132
$enabled = ($catEnabled || $rgEnabled);
11251133
if ($enabled) {

0 commit comments

Comments
 (0)