Do not escape hierarchical select

[eileenmcnaughton]

Overview

Do not escape hierarchical select

Before

Escaping applied to these quickform html hierarchical select elements, breaking the script

After

Elements marked for 'smarty:nodefaults' so escaping is not applied

Technical Details

We already have 'cutouts' in the escape function to not escape assigned html.
It isn't catching the hierarchical select elements - so I have marked a bunch
to not be escaped. I could have handled in 'escape' but I think tpl handling
is actually correct - the cutouts in the escape function was to get it to
a more manageable level

Comments

[civibot]

(Standard links)

• If this is your first pull-request for CiviCRM, please browse CONTRIBUTING.md for information about the development and testing processes.
• If you are reviewing this pull-request, you may wish to consult the test sites and the Review Standards (long template, short template).

[colemanw]

@eileenmcnaughton wouldn't we need to do this to literally every {foo.html} in ever template? Is there any way we could make that the automatic default for any quickform html being printed?

[eileenmcnaughton]

@colemanw so most of the common patterns are being skipped here ....

civicrm-core/CRM/Core/Smarty.php

Lines 417 to 443 in 42aa581

	// 'htmlall' is the nothing-specified default.
	// Don't escape things we think quickform added.
	if (strpos($string, '<input') === 0
	|| strpos($string, '<select') === 0
	// Not handling as yet but these ones really should get some love.
	|| strpos($string, '<label') === 0
	|| strpos($string, '<button') === 0
	|| strpos($string, '<span class="crm-frozen-field">') === 0
	|| strpos($string, '<textarea') === 0
	// The ones below this point are hopefully here short term.
	|| strpos($string, '<a') === 0
	// Message templates screen
	|| strpos($string, '<span><a href') === 0
	// Not sure how big a pattern this is - used in Pledge view tab
	// not sure if it needs escaping
	|| strpos($string, ' action="/civicrm/') === 0
	// eg. Tag edit page, civicrm/admin/financial/financialType/accounts?action=add&reset=1&aid=1
	|| strpos($string, ' action="" method="post"') === 0
	// This seems to be urls...
	|| strpos($string, '/civicrm/') === 0
	// Validation error message - eg. <span class="crm-error">Tournament Fees is a required field.</span>
	|| strpos($string, '
	<span class="crm-error">') === 0
	// e.g from participant tab class="action-item" href=/civicrm/contact/view/participant?reset=1&amp;action=add&amp;cid=142&amp;context=participant
	|| strpos($string, 'class="action-item" href=/civicrm/"') === 0
	) {

I think long term we probably SHOULD put the smarty:nodefaults after all html elements - because anything we auto-escape is a potential pattern to bypass security - but in order to get operational quickly I added all the carve outs for known patterns. They don't cover the hierarchical select & I figure it makes sense to do those in the way that seems more correct to me since they are not a huge volume.

[colemanw]

Ok