Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Smarty registered classes check prevents use of class constants to avoid typo bugs in templates #1028

Open
arkonan opened this issue Jun 5, 2024 · 2 comments

Comments

@arkonan
Copy link

arkonan commented Jun 5, 2024

My team makes extensive use of PHP class constants in our Smarty templates to avoid problems with typos in logic checks. With the new requirement that all classes be registered to access them statically our templates now generate deprecation warnings for each class constant.

It would be nice if class constants references (as opposed to static method calls) did not require class registration.

Alternatively I would like a supported way of overriding this behavior in a security policy. For example I would expect that overriding isTrustedStaticClass() or isTrustedStaticClassAccess() in my security policy would allow me to suppress the registered class requirement. However since the check for class registration is done outside the security policy this does not work unless the security policy also registers the class before returning. Calling Smarty::registerClass() from inside my security policy currently works but does not seem like it is a supported solution to the problem.

@asmecher
Copy link
Contributor

Agreed, using class constants in templates helps us from keeping magic strings and numbers from proliferating through the codebase, and they're easy to grep for when determining the impact of a change. Having to add a ton of registerClass calls makes this painful and I don't see the risk of just allowing constants to be accessed as a general rule.

@wisskid
Copy link
Contributor

wisskid commented Jul 4, 2024

Using class constants should not trigger the notice. This is probably an unintended side effect of #880

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants