Skip to content

Fix ArrayAccess with objects as keys#4496

Merged
fabpot merged 1 commit intotwigphp:3.xfrom
gharlan:fix-object-storage
Dec 10, 2024
Merged

Fix ArrayAccess with objects as keys#4496
fabpot merged 1 commit intotwigphp:3.xfrom
gharlan:fix-object-storage

Conversation

@gharlan
Copy link
Copy Markdown
Contributor

@gharlan gharlan commented Dec 5, 2024 •
edited
Loading

fixes #4476
The bug was introduced in 3.15 via #4425 (comment)

gharlan
gharlan commented Dec 5, 2024
View reviewed changes
src/Extension/CoreExtension.php
// array
if (Template::METHOD_CALL !== $type) {
$arrayItem = \is_bool($item) || \is_float($item) ? (int) $item : $item = (string) $item;
$arrayItem = \is_bool($item) || \is_float($item) ? (int) $item : $item;
Copy link
Copy Markdown
Contributor Author

@gharlan gharlan Dec 5, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nicolas-grekas I'm not sure if there was a good reason for the string casting. So maybe it should be done somewhere else? Or is it ok to just remove the casting?

nicolas-grekas
nicolas-grekas approved these changes Dec 10, 2024
View reviewed changes
Copy link
Copy Markdown
Contributor

@nicolas-grekas nicolas-grekas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the regression test, that's what was missing.

@fabpot
Copy link
Copy Markdown
Contributor

fabpot commented Dec 10, 2024

Thank you @gharlan.

@fabpot fabpot merged commit d8a798c into twigphp:3.x Dec 10, 2024
@gharlan gharlan deleted the fix-object-storage branch December 10, 2024 15:32
@brandonkelly
Copy link
Copy Markdown
Contributor

brandonkelly commented Oct 23, 2025 •
edited
Loading

This change has introduced a separate breaking change: previously if you did obj[key] and key was an object that implemented __toString(), it would work as expected. But now you get an error, Cannot access offset of type <TypeName> on array.

I realize that ArrayAccess::offsetGet() accepts mixed $offset, but this feels like bad behavior to me?

Arrays in PHP can only be indexed by integer or string keys, and the intent of ArrayAccess is to make objects act like arrays. So it seem reasonable to assume that the “key” passed to offsetGet() is either a string on an int.

EDIT: created an issue for this: #4701

@brandonkelly brandonkelly mentioned this pull request Oct 23, 2025
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@nicolas-grekas nicolas-grekas nicolas-grekas approved these changes

Assignees

No one assigned

Labels

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

In version 3.15.0, when accessing SplObjectStorage by index (object), an error is generated

4 participants

@gharlan @fabpot @brandonkelly @nicolas-grekas