Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reading from NamespacedAttributeBag adds entries to session #27912

Closed
Gemineye opened this Issue Jul 10, 2018 · 1 comment

Comments

Projects
None yet
5 participants
@Gemineye
Copy link

Gemineye commented Jul 10, 2018

Symfony version(s) affected: 3.4.12

Description
If i call $session->get('foo/bar/name') on a clean session I will end up with data in the session.
In my opinion a "get" should not create any entries.

How to reproduce

$session->get('foo/bar/name');
dump($session->get('foo'));

I would expect to see 'null' but i get an array containing "bar => null"

@webnet-fr

This comment has been minimized.

Copy link
Contributor

webnet-fr commented Jul 10, 2018

This clearly looks a bug. It comes from this line:

Pity that we didn't have any appropriate test to spot this bug earlier.
The quickiest solution I can imagine would be to replace that line with this snippet:

if (!$writeContext) {
    $null = null;

    return $null; // return by reference forces this trick
}

$array[$part] = array();

But I would rather not use resolveAttributePath method in has and get methods, IMO that will be clearer.

I can propose a PR later on with extended tests.

Status: Reviewed

fabpot added a commit that referenced this issue Jul 13, 2018

bug #27927 [HttpFoundation] Suppress side effects in 'get' and 'has' …
…methods of NamespacedAttributeBag (webnet-fr)

This PR was submitted for the 3.4 branch but it was merged into the 2.8 branch instead (closes #27927).

Discussion
----------

[HttpFoundation] Suppress side effects in 'get' and 'has' methods of NamespacedAttributeBag

| Q             | A
| ------------- | ---
| Branch?       | 3.4
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #27912
| License       | MIT
| Doc PR        | -

As @Gemineye reported there was a bug in `get` and `has` methods of NamespacedAttributeBag. These methods accept composite names as an argument (like 'foo/bar' or 'foo/bar/baz') to reach the elements of stored arrays. Up to now these methods erroneously created entries (`->get('foo/bar')` created `['foo' => null]`, `->get('foo/bar/baz')` created `['foo' => ['bar' => null]]`).

Commits
-------

5f59ad4 suppress side effects in 'get' or 'has' methods of NamespacedAttributeBag

@fabpot fabpot closed this Jul 13, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.