NotificationsYou must be signed in to change notification settings
Fork9.7k
Star30.8k

Fix PropertyAccessor modifying array in object when array key does no…#16090

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Jump to bottom

Merged

fabpot merged 1 commit intosymfony:2.3frompierredup:property_access

Oct 5, 2015

Merged

Fix PropertyAccessor modifying array in object when array key does no…#16090

fabpot merged 1 commit intosymfony:2.3frompierredup:property_access

Oct 5, 2015

Conversation

Copy link

Contributor

pierredup commentedOct 2, 2015

Q	A
Bug fix?	yes
New feature?	no
BC breaks?	no
Deprecations?	no
Tests pass?	yes
Fixed tickets	#16056
License	MIT
Doc PR

carsonbot added the Status: Needs Review label

Oct 2, 2015

Copy link

Contributor

Tobion commentedOct 2, 2015

👍 Looks good. But applying this in 2.7 will be tricky because we cannot just skip the last item as we also support throwing aNoSuchIndexException inhttps://github.com/symfony/symfony/blob/2.8/src/Symfony/Component/PropertyAccess/PropertyAccessor.php#L234

@pierredup do you have an idea how to best solve it in 2.7?

Copy link

Contributor

Tobion commentedOct 2, 2015

I think the best is to just move the test within the outer if in 2.7

if ($i + 1 < $propertyPath->getLength()) {    $objectOrArray[$property] = array();}

And ensure we have a test covering this when usingthrowExceptionOnInvalidIndex = true

Fix PropertyAccessor modifying array in object when array key does no…

f24c678

…t exist

pierredup force-pushed theproperty_access branch from255747d tof24c678Compare

October 3, 2015 20:00

Copy link

ContributorAuthor

pierredup commentedOct 3, 2015

@Tobion I've just tested this on the 2.7 branch, and it works by wrapping the assignment in theif.
I've updated this PR to use that approach instead, to make the merging into 2.7 a bit easier.
When this is merged then I'll create more tests to check for theNoSuchIndexException

Copy link

Member

fabpot commentedOct 5, 2015

Thank you@pierredup.

fabpot merged commitf24c678 intosymfony:2.3

Oct 5, 2015

fabpot added a commit that referenced this pull request

Oct 5, 2015

bug#16090Fix PropertyAccessor modifying array in object when array …

58bf830

…key does no… (pierredup)This PR was merged into the 2.3 branch.Discussion----------Fix PropertyAccessor modifying array in object when array key does no…| Q             | A| ------------- | ---| Bug fix?      | yes| New feature?  | no| BC breaks?    | no| Deprecations? | no| Tests pass?   | yes| Fixed tickets |#16056| License       | MIT| Doc PR        |Commits-------f24c678 Fix PropertyAccessor modifying array in object when array key does not exist

pierredup mentioned this pull request

Oct 6, 2015

Added more tests for PropertyAccess#16155

Merged

fabpot added a commit that referenced this pull request

Oct 7, 2015

minor#16155Added more tests for PropertyAccess (pierredup)

a954fea

This PR was merged into the 2.7 branch.Discussion----------Added more tests for PropertyAccess| Q             | A| ------------- | ---| Bug fix?      | no| New feature?  | no| BC breaks?    | no| Deprecations? | no| Tests pass?   | yes| Fixed tickets || License       | MIT| Doc PR        |This is a follow up for [16090#issuecomment-145183635](#16090 (comment))Commits-------378db75 Added more tests for PropertyAccess