smarty-php / smarty

Smarty is a template engine for PHP, facilitating the separation of presentation (HTML/CSS) from application logic.
Other
2.25k stars 709 forks source link

unescape:htmlall and escape:htmlall are failing with an array input #876

Closed Alaa-MK closed 1 year ago

Alaa-MK commented 1 year ago

It looks like smarty/smarty:4.3.0 broke the unescape:htmlall modifier as well as escape:htmlall when used with an array input (breaking change: here). This is because html_entity_decode doesn’t accept an array as the first argument and throws a type error. This might also apply to other modifiers that I’m not aware of.

PHP Vesion: 8.1

Error Message:

Fatal error: Uncaught TypeError: html_entity_decode(): Argument #1 ($string) must be of type string, array given

How to reproduce the issue

Here is the code I used to reproduce the issue:

// templates/test.tpl
{$data|unescape:htmlall|json_encode}
<?php

$this->smarty->data = ['test_1', 'test_2'];
$this->smarty->page = "templates/test";

Can we get a fix for this, please? 🙏 Thank you!

ampmonteiro commented 1 year ago

your $data is array type and unescape requires a string. Please read the error.

unescape docs

And for passing data you should use assign method and for templates use display method

wisskid commented 1 year ago

I don't think |escape and |unescape were ever intended to work on arrays. But they did, and the documentation isn't clear on this. So I feel we should support this again, at least in v4, in order to maintain backwards compatibility as much as possible.

I'll work on a fix.

wisskid commented 1 year ago

It wasn't intended for sure. In fact, for half of the escape formats, arrays never worked. Also didn't work if Smarty was running in multibyte mode.

@Alaa-MK I'm wondering if it's really worth to fix this. Would if be hard for you to switch to a {for} loop or create a array_escape and array_unescape modifier plugin?

Alaa-MK commented 1 year ago

@wisskid Thanks for taking a look at this! The codebase is huge, so we cannot refactor and switch to a for loop. We avoided upgrading the package for the time being, but I assume we can implement a modifier plugin if needed in the future.