[Flow] Fluid escaping interceptor not called when rendering view helpers with shorthand syntax

[Bastian Waidelich]

Bastian Waidelich wrote:

Oops, that wasn't meant to be sent yet. Hello weekend ;)

> Apart from that we should try to communicate [...]

...that one should strive for *one* context per template.
If you look at the escaping contexts from that ctemplate engine, there 
aren't too many different ones: 
http://google-ctemplate.googlecode.com/svn/trunk/doc/auto_escape.html

And most of them are considered bad practice (like inline styles and 
scripts), so hopefully the situation improves naturally.


>>>> <p style="color: {obj.color};" title="{obj.title}">;
>>>>
>>>> {obj.color} -> wrong escaping
>>>
>>> Why would that be wrong?
>>
>> This is one of the many HTML contexts where htmlspecialchars encoding is
>> not enough, since ";:" are not encoded, you can apply all styles you
>> want and you can do a lot with CSS3 nowadays ;)

Hopefully more and more people avoid these inline context-switches that 
mixes style and presentation (and in this case even the domain model) 
with each other.
S.th. like

<p class="type-{obj.type}">

...

.type-sometype { color: red }

is maybe slightly more work at first, but it quickly pays off.


>>> I'm not trying to defend Fluid here [...]

>> It (documenting implications and behavior) is not about if Fluid is good
>> or bad, but what it does when and what it does not.

I know, I know, I just mentioned it to avoid any misconception


>>> There are nasty exceptions though:
>>>
>>> {string -> f:format.crop(maxCharacters: 10)} // escaping active
>>> {f:format.crop(value: string, maxCharacters: 10)} // escaping inactive!
>>
>> Wow. While I understand this from a technical point of view, from
>> (repeating myself) a user perspective this is impossible to grasp. At
>> least it is very likely to do wrong.

Absolutely, and in this case it's certainly a bug in the crop VH

>>> This is consistent in the technical sense, but it is obviously very
>>> error-prone and I suggest to fix those rare cases by applying the
>>> escaping manually where applicable.
>>
>> I don't have an overview what places exactly you mean, but more
>> consistency in that regard would be good.

I agree, and for a VH-author it's pretty straight-forward luckily:

* Arguments are not intercepted (= escaped)
* The result of renderChildren() is by default escaped, but this can be 
disabled by setting $escapingInterceptorEnabled = FALSE;
* The result of the render() method is not intercepted

That has two consequences for the developer:

1) If you want to access the un-altered child nodes, set the 
escapingInterceptorEnabled flag to FALSE
2) Make sure the result of your VH is safe

In the case of the crop-VH we violated both and instead of

public function render($maxCharacters, $append = '...', $value = NULL) {
	if ($value === NULL) {
		$value = $this->renderChildren();
	}

	if (strlen($value) > $maxCharacters) {
		return substr($value, 0, $maxCharacters) . $append;
	} else {
		return $value;
	}
}

the code should look like:

protected $escapingInterceptorEnabled = FALSE;

public function render($maxCharacters, $append = '...', $value = NULL) {
	if ($value === NULL) {
		$value = $this->renderChildren();
	}

	if (strlen($value) > $maxCharacters) {
		$value = substr($value, 0, $maxCharacters) . $append;
	}
	return htmlspecialchars($value, ENT_COMPAT, 'UTF-8');
}


I'll go through the Fluid ViewHelpers next week and fix those 
inconsistencies. And if I get around it, I can maybe improve the 
templating section of the flow documentation to emphasize the pitfalls.


>> Would be great if we could discuss these issues in person. When is the
>> next code sprint?

Yeah! Maybe we can organize an online meeting sometimes?


-- 
Bastian Waidelich