Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

[HttpFoundation] Fixed bug in key searching for NamespacedAttributeBag #7586

Merged
merged 1 commit into from

4 participants

@MaxVandervelde
Q A
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets #7564
License MIT
Doc PR N/A

Fixed a bug in NamespacedAttributeBag causing a result to be falsely found when
the last key of the attribute matched the last of the queried name regardless of
if the key did not exist in the search.
Added Tests to demonstrate the issue and resolved by setting keys to null when
iterating through query and returning proper responses in the case that the
given array does in fact not exist.

...undation/Session/Attribute/NamespacedAttributeBag.php
@@ -46,6 +46,10 @@ public function has($name)
$attributes = $this->resolveAttributePath($name);
$name = $this->resolveKey($name);
+ if ($attributes == null) {
@stof Collaborator
stof added a note

this should be a strict comparison and it should be null === $attributes

You're right, good catch. I'll update.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
...undation/Session/Attribute/NamespacedAttributeBag.php
@@ -120,9 +128,9 @@ protected function &resolveAttributePath($name, $writeContext = false)
unset($parts[count($parts)-1]);
foreach ($parts as $part) {
- if (!array_key_exists($part, $array)) {
+ if ($array != null && !array_key_exists($part, $array)) {
@fabpot Owner
fabpot added a note

When comparing to null, you should use !==. Also, we use null !== $array in Symfony.

Thanks, I'll update this as well.

Fixed!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
...undation/Session/Attribute/NamespacedAttributeBag.php
((6 lines not shown))
if (!$writeContext) {
- return $array;
+ $array[$part] = null;
@pborreli
pborreli added a note

i don't get it, you set it to null, and immediately after it's overridden by array()

Looks like we're missing a unit test.
I updated it to include an else bracket that should have always been here. the array fill in the else is for the write context only.

@pborreli
pborreli added a note

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@MaxVandervelde MaxVandervelde [HttpFoundation] Fixed bug in key searching for NamespacedAttributeBag
Fixed a bug in NamespacedAttributeBag causing a result to be falsely found when
the last key of the attribute matched the last of the queried name regardless of
if the key did not exist in the search.
Added Tests to demonstrate the issue and resolved by setting keys to null when
iterating through query and returning proper responses in the case that the
given array does in fact not exist.

* Updated Syntax of null checks
* Fixing missing else case for if statement in write context
0f0c29c
@fabpot fabpot referenced this pull request from a commit
@fabpot fabpot merged branch MaxVandervelde/fix/namespaced-parameter-issue (PR #7586)
This PR was merged into the 2.1 branch.

Discussion
----------

[HttpFoundation] Fixed bug in key searching for NamespacedAttributeBag

| Q             | A
| ------------- | ---
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #7564
| License       | MIT
| Doc PR        | N/A

Fixed a bug in NamespacedAttributeBag causing a result to be falsely found when
the last key of the attribute matched the last of the queried name regardless of
if the key did not exist in the search.
Added Tests to demonstrate the issue and resolved by setting keys to null when
iterating through query and returning proper responses in the case that the
given array does in fact not exist.

Commits
-------

0f0c29c [HttpFoundation] Fixed bug in key searching for NamespacedAttributeBag
0d32445
@fabpot fabpot merged commit 0f0c29c into symfony:2.1
@mmucklo mmucklo referenced this pull request from a commit
@fabpot fabpot merged branch MaxVandervelde/fix/namespaced-parameter-issue (PR #7586)
This PR was merged into the 2.1 branch.

Discussion
----------

[HttpFoundation] Fixed bug in key searching for NamespacedAttributeBag

| Q             | A
| ------------- | ---
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #7564
| License       | MIT
| Doc PR        | N/A

Fixed a bug in NamespacedAttributeBag causing a result to be falsely found when
the last key of the attribute matched the last of the queried name regardless of
if the key did not exist in the search.
Added Tests to demonstrate the issue and resolved by setting keys to null when
iterating through query and returning proper responses in the case that the
given array does in fact not exist.

Commits
-------

0f0c29c [HttpFoundation] Fixed bug in key searching for NamespacedAttributeBag
d7ed92c
@hackzilla hackzilla referenced this pull request from a commit
@fabpot fabpot merged branch MaxVandervelde/fix/namespaced-parameter-issue (PR #7586)
This PR was merged into the 2.1 branch.

Discussion
----------

[HttpFoundation] Fixed bug in key searching for NamespacedAttributeBag

| Q             | A
| ------------- | ---
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #7564
| License       | MIT
| Doc PR        | N/A

Fixed a bug in NamespacedAttributeBag causing a result to be falsely found when
the last key of the attribute matched the last of the queried name regardless of
if the key did not exist in the search.
Added Tests to demonstrate the issue and resolved by setting keys to null when
iterating through query and returning proper responses in the case that the
given array does in fact not exist.

Commits
-------

0f0c29c [HttpFoundation] Fixed bug in key searching for NamespacedAttributeBag
5ba8bd6
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Apr 10, 2013
  1. @MaxVandervelde

    [HttpFoundation] Fixed bug in key searching for NamespacedAttributeBag

    MaxVandervelde authored
    Fixed a bug in NamespacedAttributeBag causing a result to be falsely found when
    the last key of the attribute matched the last of the queried name regardless of
    if the key did not exist in the search.
    Added Tests to demonstrate the issue and resolved by setting keys to null when
    iterating through query and returning proper responses in the case that the
    given array does in fact not exist.
    
    * Updated Syntax of null checks
    * Fixing missing else case for if statement in write context
This page is out of date. Refresh to see the latest.
View
16 src/Symfony/Component/HttpFoundation/Session/Attribute/NamespacedAttributeBag.php
@@ -46,6 +46,10 @@ public function has($name)
$attributes = $this->resolveAttributePath($name);
$name = $this->resolveKey($name);
+ if (null === $attributes) {
+ return false;
+ }
+
return array_key_exists($name, $attributes);
}
@@ -57,6 +61,10 @@ public function get($name, $default = null)
$attributes = $this->resolveAttributePath($name);
$name = $this->resolveKey($name);
+ if (null === $attributes) {
+ return $default;
+ }
+
return array_key_exists($name, $attributes) ? $attributes[$name] : $default;
}
@@ -120,12 +128,8 @@ protected function &resolveAttributePath($name, $writeContext = false)
unset($parts[count($parts)-1]);
foreach ($parts as $part) {
- if (!array_key_exists($part, $array)) {
- if (!$writeContext) {
- return $array;
- }
-
- $array[$part] = array();
+ if (null !== $array && !array_key_exists($part, $array)) {
+ $array[$part] = $writeContext ? array() : null;
}
$array = & $array[$part];
View
2  src/Symfony/Component/HttpFoundation/Tests/Session/Attribute/NamespacedAttributeBagTest.php
@@ -160,8 +160,10 @@ public function attributesProvider()
array('csrf.token/b', '4321', true),
array('category', array('fishing' => array('first' => 'cod', 'second' => 'sole')), true),
array('category/fishing', array('first' => 'cod', 'second' => 'sole'), true),
+ array('category/fishing/missing/first', null, false),
array('category/fishing/first', 'cod', true),
array('category/fishing/second', 'sole', true),
+ array('category/fishing/missing/second', null, false),
array('user2.login', null, false),
array('never', null, false),
array('bye', null, false),
Something went wrong with that request. Please try again.