Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Zend\Session\Container does not allow modification by reference #2510

Closed
zfbot opened this Issue Sep 28, 2012 · 10 comments

Comments

Projects
None yet
3 participants

zfbot commented Sep 28, 2012

Jira Information

Original Issue:ZF2-431
Issue Type:Docs: Task
Reporter:demiankatz
Created:07/27/12
Assignee:weierophinney
Components:Zend\Session

Description

Because of the "ArrayObject returns value rather than reference" issue (see, for example, https://bugs.php.net/bug.php?id=51622 ), the Zend\Session\Container does not allow certain types of modifications of properties/array elements.

Here are some unit tests and responses that demonstrate the issue:

    public function testContainerAllowsArrayModification()
    {
        $this->container['foo'] = 1;
        $this->assertEquals(1, $this->container['foo']);
        $this->container['foo']++;
        $this->assertEquals(2, $this->container['foo']);
    }

There was 1 error:

  1. ZendTest\Session\ContainerTest::testContainerAllowsArrayModification
    Indirect modification of overloaded element of Zend\Session\Container has no effect
    public function testContainerAllowsObjectModification()
    {
        $this->container->foo = 1;
        $this->assertEquals(1, $this->container->foo);
        $this->container->foo++;
        $this->assertEquals(2, $this->container->foo);
    }

There was 1 failure:

  1. ZendTest\Session\ContainerTest::testContainerAllowsObjectModification
    Failed asserting that 1 matches expected 2.

zfbot commented Sep 28, 2012

(Originally posted by: weierophinney on 07/30/12)

Interestingly, you can do the following fine:

$container += 1;

Considering that internally we're using an ArrayObject, the liklihood of getting this to work with increment and decrement operators is pretty slim. However, since increment/decrement on assignment works fine, I'd argue we should simply document those use cases.

zfbot commented Sep 28, 2012

(Originally posted by: weierophinney on 07/30/12)

Updating issue type to "documentation task".

zfbot commented Sep 28, 2012

(Originally posted by: demiankatz on 07/31/12)

Increment/decrement was just the simplest use case I could think of that caused a problem. Possibly more serious is the problem of appending to existing arrays:

    public function testContainerAllowsArrayModificationOfArrays()
    {
        $this->container['foo'] = array();
        $this->assertEquals(0, count($this->container['foo']));
        $this->container['foo'][] = 'bar';
        $this->assertEquals(1, count($this->container['foo']));
    }

    public function testContainerAllowsObjectModificationOfArrays()
    {
        $this->container->foo = array();
        $this->assertEquals(0, count($this->container->foo));
        $this->container->foo[] = 'bar';
        $this->assertEquals(1, count($this->container->foo));
    }

(These trigger errors equivalent to the ones listed in the ticket above).

I agree that ```. (You can't use += in this context -- it yields an "Unsupported operand types" fatal error).

zfbot commented Sep 28, 2012

(Originally posted by: weierophinney on 07/31/12)

Right, but, again, because it's an ArrayObject implementation internally, you can do this:

$container->foo->append('bar');

As such, I still feel this falls into a documentation issue -- we need to clarify that the container and its members follows ArrayObject behavior, which means you have to treat it as such when using it.

zfbot commented Sep 28, 2012

(Originally posted by: demiankatz on 07/31/12)

Makes sense -- thanks for the clarification. (I'm still not totally comfortable with ArrayObject behavior, but that's not ZF's problem!).

zfbot commented Sep 28, 2012

This issue was ported from the ZF2 Jira Issue Tracker at
http://framework.zend.com/issues/browse/ZF2-431

Known GitHub users mentioned in the original message or comment:
@demiankatz, @weierophinney

Contributor

mwillbanks commented Jan 14, 2013

@weierophinney any suggestions you have here? I believe this is basically the same type of issue that we had with the Session\Storage\SessionStorage when we utilized an ArrayObject. I don't believe we should change anything here but rather provide documentation as to the limitations of the Container object itself.

Owner

weierophinney commented Jan 14, 2013

@mwillbanks Yes -- documentation issue at this point, which is what I suggested previously as well.

Contributor

mwillbanks commented Jan 14, 2013

sounds good; i might be able to get to the documentation this week withstanding any other craze that might pop up.

Contributor

mwillbanks commented Feb 6, 2013

@weierophinney this can be closed with the latest commit; we'll need to do a documentation plan for php versions like noted in the ML.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment