Fix Stringable keys for ArrayAccess implementations

This commit is contained in:
Fabien Potencier
2026-08-26 20:47:28 +02:00
parent 885cbdb58f
commit f3f1649955
4 changed files with 239 additions and 41 deletions
+1 -1
View File
@@ -17,7 +17,7 @@
* Normalize destructuring variable AST nodes as assignment targets
* Fix `IntlExtension` ignoring explicit date/time formats and the `format_date`/`format_time` filters when a date formatter prototype is configured
* Add a `format_list` filter to `IntlExtension` to format a list of strings using PHP 8.5's `IntlListFormatter`
* Fix array access with a `Stringable` key coercing the key to string for `ArrayAccess` objects that use object keys (such as `SplObjectStorage`)
* Fix array access with a `Stringable` key for `ArrayObject` and `ArrayIterator` while preserving object keys for `SplObjectStorage`
* Fix duplicated macro argument names triggering a PHP fatal error instead of a `SyntaxError`
* Deprecate defining a macro more than once in the same template
* Deprecate `TemplateVariable` and `AssignTemplateVariable`; use `MacroVariable` and `AssignMacroVariable` instead
+12
View File
@@ -132,6 +132,14 @@ final class CoreExtension extends AbstractExtension
'SplStack',
'WeakMap',
];
/**
* @internal
*/
public const STRINGABLE_KEY_ARRAY_ACCESS_CLASSES = [
'ArrayIterator',
'ArrayObject',
'RecursiveArrayIterator',
];
private const DEFAULT_TRIM_CHARS = " \t\n\r\0\x0B";
@@ -1759,6 +1767,10 @@ final class CoreExtension extends AbstractExtension
}
}
if ($object instanceof \ArrayAccess && $arrayItem instanceof \Stringable && \in_array($object::class, self::STRINGABLE_KEY_ARRAY_ACCESS_CLASSES, true)) {
$arrayItem = (string) $arrayItem;
}
if (match (true) {
\is_array($object) => \array_key_exists($arrayItem = (string) $arrayItem, $object),
$object instanceof \ArrayAccess => $object->offsetExists($arrayItem),
+7 -7
View File
@@ -64,7 +64,8 @@ class GetAttrExpression extends AbstractExpression implements SupportDefinedTest
->raw('(('.$var.' = ')
->subcompile($this->getNode('node'))
->raw(') && is_array(')
->raw($var);
->raw($var)
;
if (!$env->hasExtension(SandboxExtension::class)) {
$compiler
@@ -167,7 +168,7 @@ class GetAttrExpression extends AbstractExpression implements SupportDefinedTest
$names[] = 'arguments';
}
// compileArrayKey() coerces a Stringable key; expose it so the sandbox checks __toString()
// compileArrayKey() may coerce a Stringable key; expose it so the sandbox checks __toString()
if (Template::ARRAY_CALL === $this->getAttribute('type')) {
$names[] = 'attribute';
}
@@ -176,10 +177,9 @@ class GetAttrExpression extends AbstractExpression implements SupportDefinedTest
}
/**
* Coerces a Stringable array key to string so the optimized path matches
* CoreExtension::getAttribute(): only arrays coerce the key, while ArrayAccess
* objects (e.g. SplObjectStorage) receive it untouched. Scalars are left to
* PHP's native offset coercion.
* Normalizes a Stringable array key so optimized access matches getAttribute():
* arrays and known string-keyed ArrayAccess implementations receive a string,
* while object-key implementations receive the object unchanged.
*/
private function compileArrayKey(Compiler $compiler, string $var): void
{
@@ -195,7 +195,7 @@ class GetAttrExpression extends AbstractExpression implements SupportDefinedTest
$compiler
->raw('(('.$key.' = ')
->subcompile($attribute)
->raw(') instanceof \Stringable && is_array('.$var.') ? (string) '.$key.' : '.$key.')')
->raw(') instanceof \Stringable && (is_array('.$var.') || in_array('.$var.'::class, CoreExtension::STRINGABLE_KEY_ARRAY_ACCESS_CLASSES, true)) ? (string) '.$key.' : '.$key.')')
;
}
+219 -33
View File
@@ -28,6 +28,7 @@ use Twig\Extension\CoreExtension;
use Twig\Extension\SandboxExtension;
use Twig\Loader\ArrayLoader;
use Twig\Sandbox\SecurityError;
use Twig\Sandbox\SecurityNotAllowedPropertyError;
use Twig\Sandbox\SecurityPolicy;
use Twig\Source;
use Twig\Template;
@@ -261,27 +262,46 @@ class TemplateTest extends TestCase
* @dataProvider getStrictVariablesModes
*/
#[DataProvider('getStrictVariablesModes')]
public function testArrayAccessWithStringableKeyIsConsistentAcrossStrictModes(bool $strict): void
public function testArrayWithStringableKeyIsConsistentAcrossStrictModes(bool $strict): void
{
$twig = new Environment(new ArrayLoader(['index' => '{{ array[object] }}']), [
'strict_variables' => $strict,
'autoescape' => false,
]);
$object = new class implements \Stringable {
public function __toString(): string
{
return 'string';
}
};
$key = new TemplateStringableKey();
$this->assertSame('value', $twig->render('index', ['array' => ['string' => 'value'], 'object' => $object]));
$this->assertSame('value', $twig->render('index', ['array' => ['string' => 'value'], 'object' => $key]));
}
public static function getStrictVariablesModes(): iterable
/**
* @dataProvider getStringableKeyArrayAccessContainers
*/
#[DataProvider('getStringableKeyArrayAccessContainers')]
public function testStringableKeyIsCoercedForInternalArrayAccess(bool $strict, bool $sandboxed, \ArrayAccess $data): void
{
yield 'lax' => [false];
yield 'strict' => [true];
$twig = new Environment(new ArrayLoader(['index' => '{{ data[key] }}']), [
'strict_variables' => $strict,
'autoescape' => false,
]);
$key = new TemplateStringableKey();
if ($sandboxed) {
$twig->addExtension(new SandboxExtension(new SecurityPolicy([], [], [$key::class => ['__toString']], [], []), true));
}
$this->assertSame('value', $twig->render('index', ['data' => $data, 'key' => $key]));
$this->assertSame(1, $key->toStringCalls);
}
public static function getStringableKeyArrayAccessContainers(): iterable
{
foreach (['lax' => false, 'strict' => true] as $mode => $strict) {
foreach (['unsandboxed' => false, 'sandboxed' => true] as $sandboxMode => $sandboxed) {
yield $mode.' '.$sandboxMode.' ArrayObject' => [$strict, $sandboxed, new \ArrayObject(['string' => 'value'])];
yield $mode.' '.$sandboxMode.' ArrayIterator' => [$strict, $sandboxed, new \ArrayIterator(['string' => 'value'])];
yield $mode.' '.$sandboxMode.' RecursiveArrayIterator' => [$strict, $sandboxed, new \RecursiveArrayIterator(['string' => 'value'])];
}
}
}
/**
@@ -290,47 +310,162 @@ class TemplateTest extends TestCase
#[DataProvider('getStrictVariablesModes')]
public function testArrayAccessWithObjectKeyKeepsTheObjectKey(bool $strict): void
{
$twig = new Environment(new ArrayLoader(['index' => '{{ data[object] }}']), [
$twig = new Environment(new ArrayLoader(['index' => '{{ data[key] }}']), [
'strict_variables' => $strict,
'autoescape' => false,
]);
$object = new class implements \Stringable {
public function __toString(): string
{
return 'string';
}
};
$key = new TemplateStringableKey();
$data = new \SplObjectStorage();
$data[$object] = 'value';
$data[$key] = 'value';
$this->assertSame('value', $twig->render('index', ['data' => $data, 'object' => $object]));
$this->assertSame('value', $twig->render('index', ['data' => $data, 'key' => $key]));
$this->assertSame(0, $key->toStringCalls);
}
public function testArrayAccessWithStringableKeyIsCheckedBySandbox(): void
/**
* @dataProvider getStrictVariablesModes
*/
#[DataProvider('getStrictVariablesModes')]
public function testArrayAccessLookupDoesNotRepeatOffsetChecks(bool $strict): void
{
$object = new class implements \Stringable {
public function __toString(): string
{
return 'string';
}
};
$data = ['array' => ['string' => 'value'], 'object' => $object];
$twig = new Environment(new ArrayLoader(['index' => '{{ data[key] }}']), [
'strict_variables' => $strict,
'autoescape' => false,
]);
$twig = new Environment(new ArrayLoader(['index' => '{{ array[object] }}']), ['autoescape' => false]);
$key = new TemplateStringableKey();
$data = new TemplateTrackingArrayAccess($key, false);
$this->assertSame('value', $twig->render('index', ['data' => $data, 'key' => $key]));
$this->assertSame([$key], $data->offsetExistsCalls);
$this->assertSame([$key], $data->offsetGetCalls);
$this->assertSame(0, $key->toStringCalls);
}
public function testArrayAccessDefinedTestDoesNotReadTheOffset(): void
{
$twig = new Environment(new ArrayLoader(['index' => '{{ data[key] is defined ? "yes" : "no" }}']));
$key = new TemplateStringableKey();
$data = new TemplateTrackingArrayAccess($key, false);
$this->assertSame('yes', $twig->render('index', ['data' => $data, 'key' => $key]));
$this->assertSame([$key], $data->offsetExistsCalls);
$this->assertSame([], $data->offsetGetCalls);
}
/**
* @dataProvider getStrictVariablesModes
*/
#[DataProvider('getStrictVariablesModes')]
public function testRejectedStringableArrayAccessKeyRethrowsOriginalTypeError(bool $strict): void
{
$twig = new Environment(new ArrayLoader(['index' => '{{ data[key] }}']), [
'strict_variables' => $strict,
'autoescape' => false,
]);
$key = new TemplateStringableKey();
$data = new TemplateTrackingArrayAccess($key, true);
try {
$twig->render('index', ['data' => $data, 'key' => $key]);
$this->fail('The original TypeError must be rethrown.');
} catch (RuntimeError $e) {
$this->assertSame($data->offsetExistsError, $e->getPrevious());
}
$this->assertSame([$key], $data->offsetExistsCalls);
$this->assertSame([], $data->offsetGetCalls);
$this->assertSame(0, $key->toStringCalls);
}
/**
* @dataProvider getStrictVariablesModes
*/
#[DataProvider('getStrictVariablesModes')]
public function testSandboxDoesNotAuthorizeStringPropertyForArrayAccessObjectKey(bool $strict): void
{
$twig = new Environment(new ArrayLoader(['index' => '{{ data[key] }}']), [
'strict_variables' => $strict,
'autoescape' => false,
]);
$key = new TemplateStringableKey();
$data = new TemplateTrackingArrayAccess($key, false);
$twig->addExtension(new SandboxExtension(new SecurityPolicy([], [], [$key::class => ['__toString']], [$data::class => ['string']], []), true));
$this->expectException(SecurityNotAllowedPropertyError::class);
try {
$twig->render('index', ['data' => $data, 'key' => $key]);
} finally {
$this->assertSame([], $data->offsetExistsCalls);
$this->assertSame([], $data->offsetGetCalls);
}
}
/**
* @dataProvider getStrictVariablesModes
*/
#[DataProvider('getStrictVariablesModes')]
public function testArrayWithStringableKeyIsCheckedBySandbox(bool $strict): void
{
$key = new TemplateStringableKey();
$context = ['array' => ['string' => 'value'], 'key' => $key];
$twig = new Environment(new ArrayLoader(['index' => '{{ array[key] }}']), ['strict_variables' => $strict, 'autoescape' => false]);
$twig->addExtension(new SandboxExtension(new SecurityPolicy([], [], [], [], []), true));
try {
$twig->render('index', $data);
$twig->render('index', $context);
$this->fail('The sandbox must reject the __toString() coercion of the array key.');
} catch (SecurityError $e) {
$this->assertStringContainsStringIgnoringCase('__toString', $e->getMessage());
}
$this->assertSame(0, $key->toStringCalls);
$twig = new Environment(new ArrayLoader(['index' => '{{ array[object] }}']), ['autoescape' => false]);
$twig->addExtension(new SandboxExtension(new SecurityPolicy([], [], [$object::class => ['__toString']], [], []), true));
$twig = new Environment(new ArrayLoader(['index' => '{{ array[key] }}']), ['strict_variables' => $strict, 'autoescape' => false]);
$twig->addExtension(new SandboxExtension(new SecurityPolicy([], [], [$key::class => ['__toString']], [], []), true));
$this->assertSame('value', $twig->render('index', $data));
$this->assertSame('value', $twig->render('index', $context));
$this->assertSame(1, $key->toStringCalls);
}
/**
* @dataProvider getStrictVariablesModes
*/
#[DataProvider('getStrictVariablesModes')]
public function testInternalArrayAccessWithStringableKeyIsCheckedBySandbox(bool $strict): void
{
$key = new TemplateStringableKey();
$twig = new Environment(new ArrayLoader(['index' => '{{ data[key] }}']), ['strict_variables' => $strict, 'autoescape' => false]);
$twig->addExtension(new SandboxExtension(new SecurityPolicy([], [], [], [], []), true));
try {
$twig->render('index', ['data' => new \ArrayObject(['string' => 'value']), 'key' => $key]);
$this->fail('The sandbox must reject the __toString() coercion of the ArrayAccess key.');
} catch (SecurityError $e) {
$this->assertStringContainsStringIgnoringCase('__toString', $e->getMessage());
}
$this->assertSame(0, $key->toStringCalls);
}
public function testSandboxedArrayAccessWithObjectKeyKeepsTheObjectKey(): void
{
$key = new TemplateStringableKey();
$data = new \SplObjectStorage();
$data[$key] = 'value';
$twig = new Environment(new ArrayLoader(['index' => '{{ data[key] }}']), ['autoescape' => false]);
$twig->addExtension(new SandboxExtension(new SecurityPolicy([], [], [$key::class => ['__toString']], [], []), true));
$this->assertSame('value', $twig->render('index', ['data' => $data, 'key' => $key]));
$this->assertSame(0, $key->toStringCalls);
}
public static function getStrictVariablesModes(): iterable
{
yield 'lax' => [false];
yield 'strict' => [true];
}
/**
@@ -640,6 +775,57 @@ class TemplateForTest extends Template
}
}
final class TemplateStringableKey implements \Stringable
{
public int $toStringCalls = 0;
public function __toString(): string
{
++$this->toStringCalls;
return 'string';
}
}
final class TemplateTrackingArrayAccess implements \ArrayAccess
{
public array $offsetExistsCalls = [];
public ?\TypeError $offsetExistsError = null;
public array $offsetGetCalls = [];
public function __construct(
private object $key,
private bool $rejectObjectOffset,
) {
}
public function offsetExists(mixed $offset): bool
{
$this->offsetExistsCalls[] = $offset;
if ($this->rejectObjectOffset && \is_object($offset)) {
throw $this->offsetExistsError = new \TypeError('Object offsets are not supported.');
}
return $this->key === $offset;
}
public function offsetGet(mixed $offset): mixed
{
$this->offsetGetCalls[] = $offset;
return 'value';
}
public function offsetSet(mixed $offset, mixed $value): void
{
}
public function offsetUnset(mixed $offset): void
{
}
}
class TemplateArrayAccessObject implements \ArrayAccess
{
protected $protected = 'protected';