processwire-issues icon indicating copy to clipboard operation
processwire-issues copied to clipboard

InputfieldTextTags missing escaping for values when using AJAX (e.g. in Repeater context)

Open nicoknoll opened this issue 10 months ago • 1 comments

I have a list of users, one is called "O'Conner". When I try to add a RepeaterItem that contains a field using InputfieldTextTags I get the following error:

VM283:1 Uncaught SyntaxError: Unexpected identifier 'Conner'
    at eval (<anonymous>)
    at JqueryCore.js?v=1.12.4:1:3708
    at jQuery.globalEval (JqueryCore.js?v=1.12.4:1:3727)
    at domManip (JqueryCore.js?v=1.12.4:1:82513)
    at jQuery.append (JqueryCore.js?v=1.12.4:1:85126)
    at addRepeaterItem (InputfieldRepeater.js?v=111-1740217539:520:18)
    at Object.success (InputfieldRepeater.js?v=111-1740217539:588:4)
    at fire (JqueryCore.js?v=1.12.4:1:43765)
    at Object.fireWith [as resolveWith] (JqueryCore.js?v=1.12.4:1:44935)
    at done (JqueryCore.js?v=1.12.4:1:134643)

I debugged this and the core issue lies here:

if($config->ajax && !empty($opts['cfgName'])) {
	// when renderReady was called during non-ajax, it may not have included the current value
	// as would be the case when there are TextTags fields in repeaters, PageEditChildren, etc.
	// so we add them here to the JS ProcessWire.config as part of the output
	$script = 'script';
	$cfgName = $opts['cfgName'];
	$out .= "<$script>";
	foreach($tags as $val => $label) {
		 $out .= "ProcessWire.config." . $cfgName . "['$val'] = '$label';";
	}
	$out .= "</$script>";
}

$label is unescapped when added, therefore ' does break the parsing. This might even lead to code injection issues using that field.

My current workaround is by changing that line to:

$escapedLabel = addcslashes($label, "'\\");
$out .= "ProcessWire.config." . $cfgName . "['$val'] = '$escapedLabel';";

but in general I'm not sure why we're not just json encoding all options

nicoknoll avatar Feb 22 '25 10:02 nicoknoll

Thanks @nicoknoll I've pushed a fix for this. Good to see you btw!

ryancramerdesign avatar Feb 28 '25 19:02 ryancramerdesign

@nicoknoll, can we close this issue?

matjazpotocnik avatar Jul 28 '25 18:07 matjazpotocnik