8000 Reading from NamespacedAttributeBag adds entries to session · Issue #27912 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

Reading from NamespacedAttributeBag adds entries to session #27912

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
Closed
Gemineye opened this issue Jul 10, 2018 · 1 comment
Closed

Reading from NamespacedAttributeBag adds entries to session #27912

Gemineye opened this issue Jul 10, 2018 · 1 comment
Labels
Bug HttpFoundation Status: Reviewed

Comments

@Gemineye
Copy link

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
Copy link
Contributor
webnet-fr commented Jul 10, 2018 •
edited
Loading

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

symfony/src/Symfony/Component/HttpFoundation/Session/Attribute/NamespacedAttributeBag.php

Line 127 in 3cfdc9e

$array[$part] = $writeContext ? array() : null;

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

@webnet-fr webnet-fr mentioned this issue Jul 11, 2018
Merged
fabpot added a commit that referenced this issue Jul 13, 2018
@fabpot
bug #27927 [HttpFoundation] Suppress side effects in 'get' and 'has' …
e2c0239 
…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 as completed Jul 13, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Bug HttpFoundation Status: Reviewed
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
5 participants
@fabpot @Gemineye @xabbuh @carsonbot @webnet-fr
0