Why Smarty is dropping support for the use of unregistered callables (such as PHP functions) in templates

[wisskid]

Starting from Smarty v5.0.0, it is no longer possible to use unregistereded callables (such as PHP functions) in templates as modifiers or as functions in expressions. To use the callables, you need to register them first. Preferably in an Extension or in the old-fashioned way by using $smarty->registerPlugin("modifier", "strrev", "strrev").

To assist users in making the transition to Smarty 5, Smarty 4.5 has introduced deprecation notices to warn users when they are using unregistereded callables in their templates. Deprecation notices are not errors and they don't mean your code is broken (yet). They are helpful to assist you in making the transition to Smarty 5, should you ever wish to do so. Read more about dealing with deprecations if you want to learn more.

But why?

Supporting changes in PHP

Some people have expressed frustrations about the decision to drop support for unregistered callables and wonder why Smarty is doing this. This is discussed in length in Issues and PRs, such as #813. To summarize: many users have filed bug reports to "fix" compatibility issues in modifiers that were never part of the Smarty language. For example: in PHP's join or implode it used to be possible to switch separator and array. When using join or implode as a modifier, it makes sense to switch them for readability. However, since PHP8.0 this is no longer allowed. Is this a bug in Smarty now?

Security en separation of concerns

Not relevant for everybody, but supporting each and every callable can be a nightmare when you need a strict separation between front-end developers (possibly even anonymous users) and PHP-developers. Allowing callables means you can do funny stuff like:

{"pwd"|passthru}
{"whoami"|passthru}

And yes, Smarty has security features that are meant to block stuff like this, but we felt it was better to stop this at the beginning.

Stability

One of the goals of Smarty is to provide a stable interface. This means we try to have automated testing for every language element for every supported version of PHP. If we do not somehow limit the amount of functions template authors can use, this is impossible.

Other template engines, such as Twig for example, follow the same principle.

A dirty fix

If writing a proper Extension feels as too much work, we have an easy fix for Smarty 5.

class WildcardExtension extends \Smarty\Extension\Base {

	public function getModifierCallback(string $modifierName) {
		if (is_callable($modifierName)) {
			return $modifierName;
		}
		return null;
	}

}

and

$smarty->addExtension(new WildcardExtension());

It's not pretty, but at least this makes it rather clear who is responsible for supporting the callables.

[olelis]

Thank you for starting this as a discussion!

If I understand correctly, then the main reasons are:

1. Some user are complaining that when some function in the php changed order/logic, and now Smarty is not working(even if it is not related to smarty at all)
2. Security point - so there will be some way of limiting what functions frontend developers can use

Both are valid points, but I do afraid that this solution raises more problems that it solves.

For example, when this update will be implemented, then it will mean that there will be even more complains from users that now Smarty is not working at all (and this time it will be Smarty, not PHP version change). Ok, there is a workaround, but it means that every developer in the world will have to find this workaround and copypaste it to the code. Is it worth it?

Also, I am not sure that the main behavior is that only limited list of functions is allowed. During the years (I still have my project where I have used Smarty around 2010) , developers are got used to be able to use native/userland functions directly in the Smarty code. Is it good/bad ? Hard to say.

Also, it will be quite hard to find all functions used, especially for the old, large codebase, which means that wildcard solution would be de facto standard for the old code.

Other platforms

I have checked Laravel Blade, and looks it actually supports native/userland functions out of the box. For example:
{{mt_rand(1,200)}} {{file_get_contents('test.txt')}}
is converted to
<?php echo e(mt_rand(1,200)); ?><?php echo e(file_get_contents('test.txt')); ?>

Looks like Twig does not natively support it, correct me if I am wrong.

Proposed solution

Should this new behavior be optional, similar how muteUndefinedOrNullWarnings was implemented?
So if I want to use "secure" way, then I can opt-in for it. Otherwise, I will be able to use all functions .
This will not help with the issue 1, you raised, but it will allow to have sandboxed Smarty, which is good

This solution is kinda like implementing "dirty solution" you have above, but inside Smarty itself. This will simplify life of many developers in the world.

[wisskid]

It's not that hard to find them. Several applications have alreay done so. Someone has even developed a regex that you can use in your IDE to search. Also, this is where the depreciation notices come in handy.

And if all of that is not sufficient for your usecase, you can even create a wildcard extension that throws warnings.

[olelis]

@wisskid, it Is still extra work, that should be repeated for all existing project. And this means that every developer will have to find workaround OR find all such functions. I still don't see why you force all developers to do this work.

However, based on this and other discussions, looks like your decision is final and you really think that this is the only option. In other words, looks like there is no points of discussion.,

[lkppo]

that should be repeated for all existing project. And this means that every developer will have to find workaround OR find all such functions. I still don't see why you force all developers to do this work.

However, based on this and other discussions, looks like your decision is final and you really think that this is the only option. In other words, looks like there is no points of discussion.,

Do you say the same thing when the authors of PHP introduce incompatible modifications with each new version, each year? To a technical decision you respond with feelings. Place your arguments at the same level if you want a chance to debate.

These modifications are justified, documented and announced. It's great work.

[olelis]

To a technical decision you respond with feelings.

Actually not. My comment about that there is no room on to discussion is based on reading other issue, for example #813 (I did found it between my first and second post). In the issue, similar concerns/questions were raised in Jan-Jun 2023, without affecting anything.

Also, there are already tickets that that are based on the fact that Smarty don't allow native/userland functions by default. For example #896, #959, #961. And I do believe, that there will be more of them (well, strictly speaking, this sentence is feeling, not a fact, I can't predict the future).

Do you say the same thing when the authors of PHP introduce incompatible modifications with each new version, each year?
Sometimes I don't like the changes, but at least I can always find solid reasoning why the change is done and long discussion with voting about pros and cons. This is especially true if the change actually affects a lot of people.

In addition to that, I see that quite a lot of PHP changes are rejected based on the "backward compatibility" issue. PHP developers actually try to make sure that the old code works without major changes, if this even possible. I would say that in the since php 7.4, the only major change that actually broke functionality (for me) was about strict string comparisons, other changes didn't break anything/much.

[Ninjinka]

First af all thanx for the hard work, we use smarty since the first release and followed the development and sometimes contribuited too (before the current team take the lead).

This i think is a major issue, we use smarty in around 200 projects.
Commonly 1 time every year we update our main library for all the projects, this includes every composer plugin.
Of course we cannot follow every single library decision, we "hope" they mantein retro-compatibility or made only small changes every time so we can adjust everything without giving big downtime.

This huge changes broke completely every single application so we should stay on old version since we cannot use 3-4 weeks time following every single change and also making single modifier for every single php function.

We will wait some time but if retro-compatibility disappear we should search for a new library or develop our own or maybe fork the latest working release, and i think many will do same..

Sorry to have bothered with this reflection..

[ssigwart]

That's good news.

In our case, assign cannot be used as a direct replacement of assignByRef. What we do is something like this (with much cleaner and safer code) in many places:

$originalFormData = [
	'field1' => 'abc'
];
$tpl->assignByRef('originalFormData', $originalFormData);

if (isHttpPost())
{
	$originalFormData['field1'] = $_POST['field1'];
}

By doing this, if there's a form error and we don't redirect the user, the TPL file will refill the template with the data the user entered.

These are fairly easy to fix by switching to assign and putting that below the POST handling, but still requires tons of testing.

Our problem is that we have other places where we assign array class properties by reference. So anywhere in the class or subclasses we want to add a field to that array, it will automatically be available in the TPL file. So those aren't easy fixes. I'll give you that it's not ideal code, but we've been using Smarty for 15 years, so we've got some stuff that we wouldn't do today, but trying to find and clean up the old code just isn't feasible.

[wisskid]

I see. That would complicate things a bit indeed. Maybe you could try using an ArrayObject instead? That would behave like an object but be passed by reference by default.

Meanwhile, I think #964 should be fixed by #985.

[ssigwart]

That's a good suggestion, but didn't seem to work when I tried this:

$this->tpl->assign('myArray', new ArrayObject($this->myArray));

It doesn't seem to be by reference, so the TPL file doesn't see changes after that.

I could potential change the type of the class parameter to ArrayObject, but that has it's own set of issues in that I still need to update too many spots that use it including function type hints.

One workable solution might be to create my own ArrayObjectByRef class, but we also use it for nullable fields and non-array fields too.

[wisskid]

This is what I had in mind and what it outputs:

class DummySmarty  {
	private $data = [];
	public function assign(string $varname, $value)	{ $this->data[$varname] = $value; }
	public function getVariable(string $varname) { return $this->data[$varname]; }
}

$smarty = new DummySmarty();
$formDataArray = ['a' => 'A', 'b' => 'B'];
$formDataObject = new ArrayObject($formDataArray);
$smarty->assign('formdata', $formDataObject);

// pre-change
var_dump($smarty->getVariable('formdata'));

$formDataObject['b'] = 'C';

// post-change
var_dump($smarty->getVariable('formdata'));

output

object(ArrayObject)#2 (1) {
  ["storage":"ArrayObject":private]=>
  array(2) {
    ["a"]=>
    string(1) "A"
    ["b"]=>
    string(1) "B"
  }
}
object(ArrayObject)#2 (1) {
  ["storage":"ArrayObject":private]=>
  array(2) {
    ["a"]=>
    string(1) "A"
    ["b"]=>
    string(1) "C"
  }
}

[ssigwart]

Yes, that would work if we could swap the array for ArrayObject, but that's not really viable for us. We use type hints extensively, so if we swapped out our array property for an ArrayObject and then tried calling, function doSomething(array $arr): void, we'd get a TypeError.

[marclaporte]

we use smarty in around 200 projects

Wow, this is huge!

[KarelWintersky]

However, since PHP8.0 this is no longer allowed.

Why? What change in PHP 8 prohibits the use of this mechanism? I don’t see any prohibitions in the changelog:

https://php.watch/versions/8.0

[KarelWintersky]

However, if these changes allow you to quickly implement other PHP 8 features that I requested - well, say that “this is the price of future changes” and I will accept that the inconvenience in one place will outweigh the convenience in another.

I'm talking about:
#864

[Ninjinka]

Hello!
You are limiting the use of php function, why in the world you should test for example nl2br or stripslashes or array_pop, and why i should not be allowed to use 'em? And why should i use a "dirty fix" if the engine need a fix release it...

If you want to "limiting the amount logic that can be used in templates" you should block the creation of new variables and data modification too (so the = symbol)

[KarelWintersky]

@Ninjinka I have a guess what the true reasons for such changes may be, but they lie in the field of psychology and go beyond our professional sphere. Not being a professional in the field of psychoanalysis, I will not divulge them. I hope I'm wrong and this isn't the reason.

[wisskid]

This isn't helpful. Lets try to keep it nice here.

[KarelWintersky]

@wisskid , sorry. Is my arguments valid ?

[KarelWintersky]

we have an easy fix for Smarty 5.

Is there a similar solution for smarty 4?

[wisskid]

Exactly, thank you. 🙏
This is what deprecations are for. Everyone uses them, including PHP itself. This is also why I included a link to https://stitcher.io/blog/dealing-with-deprecations above for more backgroud.

[KarelWintersky]

@wisskid

But we can't disable deprecation notices only for smarty (yes, knowing that the functionality is deprecated and agreeing to it), but leave these notifications for other packages.

This setting is global for the entire interpreter.

// $SMARTY->registerPlugin(Smarty::PLUGIN_MODIFIER, 'count', 'count');

$SMARTY->setErrorReporting(E_ALL & ~E_NOTICE & ~E_DEPRECATED);

Does not work with message

Using unregistered function "count" in a template is deprecated and will be removed in a future release. Use Smarty::registerPlugin to explicitly register a custom modifier.

P.S. Smarty version 4.5.2

[olelis]

@KarelWintersky , this is the same issue I had yesterday, please, add & ~E_USER_DEPRECATED as in my example above.

[KarelWintersky]

@olelis , @wisskid , absolutely not obvious :-(

[olelis]

@KarelWintersky You should blame PHP for this, not Smarty 😊
For example, pure php code:

error_reporting(E_ALL & ~E_DEPRECATED & ~E_NOTICE);
trigger_error('Deprecated',E_DEPRECATED);
trigger_error('Deprecated 2',E_USER_DEPRECATED);

First one will hidden, while second one not. I do understand some reasoning for this, but yes, it is counter-intuitive.
Nevertheless, it is not Smarty issue.