Skip to content

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.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 1 commit into from
Oct 5, 2015
Merged

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

merged 1 commit into from
Oct 5, 2015

Conversation

pierredup
Copy link
Contributor

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

@Tobion
Copy link
Contributor

Tobion commented Oct 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 a NoSuchIndexException in https://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?

@Tobion
Copy link
Contributor

Tobion commented Oct 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 using throwExceptionOnInvalidIndex = true

@pierredup
Copy link
Contributor Author

@Tobion I've just tested this on the 2.7 branch, and it works by wrapping the assignment in the if.
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 the NoSuchIndexException

@fabpot
Copy link
Member

fabpot commented Oct 5, 2015

Thank you @pierredup.

@fabpot fabpot merged commit f24c678 into symfony:2.3 Oct 5, 2015
fabpot added a commit that referenced this pull request Oct 5, 2015
@fabpot
bug #16090 Fix 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 pierredup mentioned this pull request Oct 6, 2015
Merged
fabpot added a commit that referenced this pull request Oct 7, 2015
@fabpot
minor #16155 Added 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
@kriswallsmith kriswallsmith mentioned this pull request Oct 22, 2015
Closed
This was referenced Oct 27, 2015
Closed
Merged
Merged
@pierredup pierredup deleted the property_access branch February 16, 2017 19:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
Status: Needs Review
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@pierredup @Tobion @fabpot @carsonbot