Skip to content

[Bugfix]: mark editables anywhere in Twig as "safe"#18620

Merged
fashxp merged 1 commit intopimcore:11.5from
jdreesen:bugfix/escape-editables-in-twig
Nov 6, 2025
Merged

[Bugfix]: mark editables anywhere in Twig as "safe"#18620
fashxp merged 1 commit intopimcore:11.5from
jdreesen:bugfix/escape-editables-in-twig

Conversation

@jdreesen
Copy link
Copy Markdown
Contributor

@jdreesen jdreesen commented Aug 15, 2025 •
edited
Loading

Changes in this pull request

Editables are already marked as safe in the pimocre_* Twig function definition.

But that only seems to work inside the very template the function is used.

As soon as you start passing it around, like:

{# some/template.html.twig #}

{{ include('some/include.html.twig', {
    headline: pimcore_input('headline'),
}) }}

You suddenly have to apply the raw filter, otherwise you'll get double-escaped output:

{# some/include.html.twig #}

{{ headline|raw }}

This makes it difficult, because the headline may not always be an editable, but something else that you want to escape.

So you would have to do stuff like this:

{# some/include.html.twig #}

{{ headline is instanceof('Pimcore\\Model\\Document\\Editable\\EditableInterface') ? headline|raw : headline }}

All of this would not be necessary if Twig would know that instances of Editable handle escaping themselves (like it already is for the editable functions).

Then you can just use it with auto-escaping and everything is fine:

{# some/include.html.twig #}

{{ headline }}

This is what this bug fix achieves.

@github-actions
Copy link
Copy Markdown

github-actions bot commented Aug 15, 2025

Review Checklist

  • Target branch (11.5 for bug fixes, others 11.x)
  • Tests (if it's testable code, there should be a test for it - get help)
  • Docs (every functionality needs to be documented, see here)
  • Migration incl. install.sql (e.g. if the database schema changes, ...)
  • Upgrade notes (deprecations, important information, migration hints, ...)
  • Label
  • Milestone

@jdreesen
Copy link
Copy Markdown
Contributor Author

jdreesen commented Aug 20, 2025

Is there any chance of this being considered for Pimcore 11.5.9?

fashxp
fashxp reviewed Aug 20, 2025
View reviewed changes
@jdreesen jdreesen force-pushed the bugfix/escape-editables-in-twig branch from 0304593 to c487ee9 Compare August 20, 2025 10:48
@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud bot commented Aug 20, 2025

Quality Gate Failed Quality Gate failed

Failed conditions
1 New Major Issues (required ≤ 0)
B Maintainability Rating on New Code (required ≥ A)

See analysis details on SonarQube Cloud

Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE

@jdreesen
Copy link
Copy Markdown
Contributor Author

jdreesen commented Aug 20, 2025

Sonar issue has nothing to do with this PR.

@jdreesen
Copy link
Copy Markdown
Contributor Author

jdreesen commented Sep 26, 2025

Maybe we can get this in Pimcore 11.5.10?

@jdreesen
Copy link
Copy Markdown
Contributor Author

jdreesen commented Oct 17, 2025

Any chance to get this merged into 11.5.12?

@jdreesen jdreesen requested a review from fashxp November 3, 2025 21:42
@fashxp
Copy link
Copy Markdown
Member

fashxp commented Nov 5, 2025

Do I get this right, in DocumentEditableExtension we mark 'just' the functions prefixed with pimcore_ as html safe, with the TwigEnvironmentConfigurator we make all editables, that extend our editables class html safe?

Wondering, if we might introduce some hidden security topics with this?

@jdreesen
Copy link
Copy Markdown
Contributor Author

jdreesen commented Nov 5, 2025

Well, “'just' the functions prefixed with pimcore_” actually means all editables (even custom ones), right?

Since pimcore_* is a dynamic Twig function, this means that if you register a custom editable foobar, you can/will use it in Twig via {{ pimcore_foobar(‘something’) }}, just like any native editable from Pimcore, right?

The Twig function pimcore_* calls Pimcore\Twig\Extension\DocumentEditableExtension::renderEditable(), which returns the editable instance which will then be echoed (meaning that __toString() is called).

Marking Pimcore\Model\Document\Editable (and all its children) as HTML-safe means that Twig will no longer escape it after converting it to a string (which also means that __toString() will be called).

@fashxp fashxp added this to the 11.5.13 milestone Nov 6, 2025
@fashxp fashxp added the Bug label Nov 6, 2025
@fashxp
Copy link
Copy Markdown
Member

fashxp commented Nov 6, 2025

thx very much...

@fashxp fashxp merged commit 81b41e8 into pimcore:11.5 Nov 6, 2025
18 of 19 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Nov 6, 2025
@jdreesen jdreesen deleted the bugfix/escape-editables-in-twig branch November 6, 2025 09:37
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@fashxp fashxp Awaiting requested review from fashxp

Assignees

No one assigned

Labels

Bug

Projects

None yet

Milestone

11.5.13

Development

Successfully merging this pull request may close these issues.

3 participants

@jdreesen @fashxp @astapc