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

Improvement of auto-escaping #1030

Merged
merged 5 commits into from
Jun 30, 2024
Merged

Improvement of auto-escaping #1030

merged 5 commits into from
Jun 30, 2024

Conversation

Amaury
Copy link
Contributor

@Amaury Amaury commented Jun 9, 2024

This evolution improves the auto-escaping feature.

  • The escape modifier has no effect when auto-escaping is enabled (when no escape format is given, or when the html format is used), to prevent double-escaping.
  • The other formats of the escape modifier (htmlall, url, urlpathinfo, quotes, javascript) are processed as we may expect, without double-escaping.
  • The new force format of the escape modifier allows to force double-escaping if needed.
  • The new raw modifier temporary disables auto-escaping for the expression it is used on.

This Pull Request contains the source code of this evolution, as well as its documentation and unit tests.

Amaury added 2 commits June 9, 2024 11:16
…' modifier; add the 'force' mode to the 'escape' modifier; add the 'raw' modifier.
@wisskid
Copy link
Contributor

wisskid commented Jun 19, 2024

This is looking great, @Amaury !

How do you propose we release this? Do you think this calls for a new major version, i.e. Smarty v6 or a minor (v5.4)?

One might argue this requires a major, because behavior is changing for existing templates that use |escape of some form when html auto-escaping is also enabled. On the other hand: that would very likely be unintended anyway.

@Amaury
Copy link
Contributor Author

Amaury commented Jun 19, 2024

About the release, I would say that a minor version should be enough; I don't know if anybody if really using auto-escaping and |escape at the same time. On the other hand, it's true this is a deep change of behaviour…
I don't know, I let you choose ^^'

src/Smarty.php Outdated Show resolved Hide resolved
@wisskid wisskid self-assigned this Jun 30, 2024
@wisskid wisskid merged commit 2289fa6 into smarty-php:master Jun 30, 2024
11 checks passed
@Amaury
Copy link
Contributor Author

Amaury commented Jul 11, 2024

Thank you! :)

@Amaury
Copy link
Contributor Author

Amaury commented Jul 11, 2024

In the end, do you think you will create a major version or a minor one?

@wisskid
Copy link
Contributor

wisskid commented Jul 11, 2024

I'm thinking minor.

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

Successfully merging this pull request may close these issues.

2 participants