diff --git a/CHANGELOG b/CHANGELOG index be9573bd9..af3ab792b 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -15,6 +15,7 @@ * Speed up loading compiled templates that OPcache already holds * Speed up applying `html_cva()` recipes that resolve to classes already seen * Fix a class marked safe for `html_attr` not being considered safe in `html` and `html_attr_relaxed` contexts + * Add a way to mark a `Twig\Markup` instance as safe for some escaping strategies only # 3.30.0 (2026-09-25) diff --git a/doc/api.rst b/doc/api.rst index 53c93000a..9b87e8122 100644 --- a/doc/api.rst +++ b/doc/api.rst @@ -558,6 +558,30 @@ The escaping rules are implemented as follows: and ``html``, and a class marked safe for ``html_attr_relaxed`` is also safe for ``html``, as the stricter escaping covers the looser contexts. +* A single value can be marked as safe for some strategies when it is created, + without registering its class: + + .. versionadded:: 3.31 + + The third argument of the ``Twig\Markup`` constructor was added in Twig + 3.31. + + .. code-block:: php + + // safe for the HTML strategy only, escaped everywhere else + $value = new \Twig\Markup($html, 'UTF-8', ['html']); + + // safe for the HTML and JS strategies + $value = new \Twig\Markup($html, 'UTF-8', ['html', 'js']); + + // safe for all strategies, the default + $value = new \Twig\Markup($html, 'UTF-8'); + + Mark values for the strategies they were really produced for: a value built + for an HTML page is not safe to inject into a JavaScript or CSS context. The + strategies of the instance always win over the ones registered for its class, + and an explicit ``|escape('html')`` in a template escapes the value anyway. + * Escaping is applied before printing, after any other filter is applied: .. code-block:: twig diff --git a/src/Extension/CoreExtension.php b/src/Extension/CoreExtension.php index 71a4be2d4..410597bbb 100644 --- a/src/Extension/CoreExtension.php +++ b/src/Extension/CoreExtension.php @@ -1218,8 +1218,8 @@ final class CoreExtension extends AbstractExtension default => throw new RuntimeError('Trimming side must be "left", "right" or "both".'), }; - // trimming a safe string with the default character mask always returns a safe string (independently of the context) - return $string instanceof Markup && self::DEFAULT_TRIM_CHARS === $characterMask ? new Markup($trimmed, $string->getCharset()) : $trimmed; + // trimming a safe string with the default character mask keeps it as safe as it was + return $string instanceof Markup && self::DEFAULT_TRIM_CHARS === $characterMask ? $string->withContent($trimmed) : $trimmed; } /** diff --git a/src/Markup.php b/src/Markup.php index 820f09143..ec2e5e22f 100644 --- a/src/Markup.php +++ b/src/Markup.php @@ -11,13 +11,16 @@ namespace Twig; +use Twig\Runtime\EscaperRuntime; + /** * Marks a content as safe. * * Instances of this class (and existing subclasses) are trusted by the Twig - * sandbox: method calls and property accesses on a Markup instance bypass the - * SecurityPolicy method/property allowlists. This is by design: Markup - * represents content that has already been deemed safe to output. + * sandbox: method calls (except withContent()) and property accesses on a + * Markup instance bypass the SecurityPolicy method/property allowlists. This + * is by design: Markup represents content that has already been deemed safe + * to output. * * This class is considered final as of Twig 3.28 and will be final in Twig * 4.0. @@ -31,8 +34,14 @@ class Markup implements \Countable, \JsonSerializable, \Stringable private $content; private ?string $charset; - public function __construct($content, $charset) - { + /** + * @param string[] $safeStrategies The escaping strategies the content is safe for, `['all']` for all of them + */ + public function __construct( + $content, + $charset, + private array $safeStrategies = ['all'], + ) { $this->content = (string) $content; $this->charset = $charset; } @@ -47,6 +56,28 @@ class Markup implements \Countable, \JsonSerializable, \Stringable return $this->charset; } + /** + * @internal + */ + public function withContent(string $content): self + { + $markup = clone $this; + $markup->content = $content; + + return $markup; + } + + public function isSafeFor(string $strategy): bool + { + foreach ($this->safeStrategies as $declared) { + if ('all' === $declared || $strategy === $declared || \in_array($strategy, EscaperRuntime::IMPLIED_STRATEGIES[$declared] ?? [], true)) { + return true; + } + } + + return false; + } + /** * @return int */ diff --git a/src/Runtime/EscaperRuntime.php b/src/Runtime/EscaperRuntime.php index 1a4efb46f..a2b0df06d 100644 --- a/src/Runtime/EscaperRuntime.php +++ b/src/Runtime/EscaperRuntime.php @@ -21,8 +21,10 @@ final class EscaperRuntime implements RuntimeExtensionInterface * Strategies a value is also safe for when it is safe for the declared one. * * Mirrors the compile-time rules of Twig\NodeVisitor\SafeAnalysisNodeVisitor. + * + * @internal */ - private const IMPLIED_STRATEGIES = [ + public const IMPLIED_STRATEGIES = [ 'html_attr' => ['html_attr_relaxed', 'html'], 'html_attr_relaxed' => ['html'], ]; @@ -116,7 +118,7 @@ final class EscaperRuntime implements RuntimeExtensionInterface public function escape($string, string $strategy = 'html', ?string $charset = null, bool $autoescape = false) { if ($autoescape && $string instanceof Markup) { - return $string; + return $string->isSafeFor($strategy) ? $string : $this->escape((string) $string, $strategy, $charset); } if (!\is_string($string)) { diff --git a/src/Sandbox/SecurityPolicy.php b/src/Sandbox/SecurityPolicy.php index f44bd3585..f29ef0e89 100644 --- a/src/Sandbox/SecurityPolicy.php +++ b/src/Sandbox/SecurityPolicy.php @@ -146,12 +146,13 @@ final class SecurityPolicy implements SecurityPolicyInterface public function checkMethodAllowed($obj, $method): void { - if ($obj instanceof Template || $obj instanceof Markup) { + $method = strtolower($method); + + if ($obj instanceof Template || ($obj instanceof Markup && 'withcontent' !== $method)) { return; } $allowed = false; - $method = strtolower($method); foreach ($this->allowedMethods as $class => $methods) { if ($obj instanceof $class && \in_array($method, $methods, true)) { $allowed = true; diff --git a/tests/Fixtures/autoescape/markup_safe_strategies.test b/tests/Fixtures/autoescape/markup_safe_strategies.test new file mode 100644 index 000000000..62f1c69bc --- /dev/null +++ b/tests/Fixtures/autoescape/markup_safe_strategies.test @@ -0,0 +1,22 @@ +--TEST-- +A "Markup" instance is only safe for the strategies it was created for +--TEMPLATE-- +{{ html }} +{{ html|trim }} +{{ safe_everywhere }} +--DATA-- +return ['html' => new \Twig\Markup('
', 'UTF-8', ['html']), 'safe_everywhere' => new \Twig\Markup('
', 'UTF-8')] +--CONFIG-- +return ['autoescape' => 'html'] +--EXPECT-- +
+
+
+--DATA-- +return ['html' => new \Twig\Markup('
', 'UTF-8', ['html']), 'safe_everywhere' => new \Twig\Markup('
', 'UTF-8')] +--CONFIG-- +return ['autoescape' => 'js'] +--EXPECT-- +\u0020\u003Cbr\u0020\/\u003E\u0020 +\u003Cbr\u0020\/\u003E +
diff --git a/tests/MarkupTest.php b/tests/MarkupTest.php new file mode 100644 index 000000000..2b73c4398 --- /dev/null +++ b/tests/MarkupTest.php @@ -0,0 +1,53 @@ +assertTrue((new Markup('
', 'UTF-8'))->isSafeFor('js')); + } + + /** + * @dataProvider provideSafeStrategies + */ + #[DataProvider('provideSafeStrategies')] + public function testContentIsSafeForTheGivenStrategies(bool $safe, array $safeStrategies, string $strategy): void + { + $this->assertSame($safe, (new Markup('
', 'UTF-8', $safeStrategies))->isSafeFor($strategy)); + } + + public static function provideSafeStrategies(): iterable + { + yield [true, ['all'], 'js']; + yield [true, ['html', 'js'], 'js']; + yield [false, ['html', 'js'], 'css']; + yield [true, ['html_attr'], 'html']; + yield [true, ['html_attr'], 'html_attr_relaxed']; + yield [false, ['html'], 'html_attr']; + yield [false, [], 'html']; + } + + public function testWithContentKeepsTheSafeStrategies(): void + { + $markup = (new Markup('
', 'UTF-8', ['html']))->withContent('
'); + + $this->assertSame('
', (string) $markup); + $this->assertTrue($markup->isSafeFor('html')); + $this->assertFalse($markup->isSafeFor('js')); + } +} diff --git a/tests/Runtime/EscaperRuntimeTest.php b/tests/Runtime/EscaperRuntimeTest.php index caaf068fd..db84d9b76 100644 --- a/tests/Runtime/EscaperRuntimeTest.php +++ b/tests/Runtime/EscaperRuntimeTest.php @@ -23,6 +23,7 @@ namespace Twig\Tests\Runtime; use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\TestCase; use Twig\Error\RuntimeError; +use Twig\Markup; use Twig\Runtime\EscaperRuntime; class EscaperRuntimeTest extends TestCase @@ -461,6 +462,43 @@ class EscaperRuntimeTest extends TestCase $this->assertSame('
', $escaper->escape(new Extension_TestClass(), 'html', null, true)); } + + /** + * @dataProvider provideMarkupSafeStrategies + */ + #[DataProvider('provideMarkupSafeStrategies')] + public function testMarkupIsOnlySafeForItsOwnStrategies(string $expected, ?array $safeStrategies, string $strategy): void + { + $markup = null === $safeStrategies ? new Markup('
', 'UTF-8') : new Markup('
', 'UTF-8', $safeStrategies); + + $this->assertSame($expected, (string) (new EscaperRuntime())->escape($markup, $strategy, null, true)); + } + + public static function provideMarkupSafeStrategies() + { + return [ + 'no argument stays safe everywhere' => ['
', null, 'js'], + 'all' => ['
', ['all'], 'js'], + 'current strategy' => ['
', ['html'], 'html'], + 'other strategy' => ['\u003Cbr\u0020\/\u003E', ['html'], 'js'], + 'several strategies' => ['
', ['html', 'js'], 'js'], + 'implied strategy' => ['
', ['html_attr'], 'html'], + 'no strategy' => ['<br />', [], 'html'], + ]; + } + + public function testMarkupStrategiesTakePrecedenceOverSafeClasses(): void + { + $escaper = new EscaperRuntime(); + $escaper->addSafeClass(Markup::class, ['html']); + + $this->assertSame('<br />', $escaper->escape(new Markup('
', 'UTF-8', ['js']), 'html', null, true)); + } + + public function testMarkupIsEscapedWhenTheDeveloperCallsTheEscapeFilter(): void + { + $this->assertSame('<br />', (new EscaperRuntime())->escape(new Markup('
', 'UTF-8', ['html']), 'html')); + } } function escaper($string, $charset) diff --git a/tests/Sandbox/SandboxTest.php b/tests/Sandbox/SandboxTest.php index 76ab18f63..7f5ff8eeb 100644 --- a/tests/Sandbox/SandboxTest.php +++ b/tests/Sandbox/SandboxTest.php @@ -218,6 +218,26 @@ class SandboxTest extends TestCase $this->assertSame('b', $sandbox->render('index', ['m' => new Markup('b', 'UTF-8')])); } + public function testMarkupSafeStrategiesCanBeChecked(): void + { + $sandbox = new Sandbox(self::env(['index' => '{{ m.isSafeFor("html") ? "yes" : "no" }}']), self::strictPolicy()); + + $this->assertSame('yes', $sandbox->render('index', ['m' => new Markup('b', 'UTF-8', ['html'])])); + } + + public function testMarkupWithContentIsGovernedByThePolicy(): void + { + $templates = ['index' => '{{ m.withContent("b") }}']; + + $allowing = new Sandbox(self::env($templates), self::strictPolicy(methods: [Markup::class => ['withContent']])); + $this->assertSame('b', $allowing->render('index', ['m' => new Markup('a', 'UTF-8')])); + + $denying = new Sandbox(self::env($templates), self::strictPolicy()); + $this->expectException(SecurityNotAllowedMethodError::class); + $this->expectExceptionMessage('Calling "withcontent" method on a "Twig\Markup" object is not allowed'); + $denying->render('index', ['m' => new Markup('a', 'UTF-8')]); + } + public function testIncludedTemplatesAreSandboxed(): void { $sandbox = new Sandbox(self::env([