Skip to content
This repository was archived by the owner on Jan 31, 2020. It is now read-only.

Conversation

ArchangelDesign
Copy link

Perhaps issue itself is in FlashMessenger rather than in Session but this change resolves issue on PHP 5.5 and 5.6. For some reason isset returns true but object is empty thus exception is thrown.
Regardless of PHP configuration (session autostart and storage location) this issue is reproducible. Setting storage manually also doesn't help.

mwillbanks and others added 2 commits July 22, 2015 21:35
For some reason FlashMessenger plugin doesn't work on PHP 5.5 and 5.6
storage is never created, isset returns true but $storage[$name] is empty.
Tested on Linux Ubuntu 12, 14 and Apache 2.4
@mwillbanks
Copy link
Contributor

@ArchangelDesign could you please add a test that exhibits this behavior so that we can ensure everything is good going forward?

@mwillbanks
Copy link
Contributor

@ArchangelDesign i would really like to see if we can get this done today as the plan is to release the next version at some point today.

@mwillbanks
Copy link
Contributor

@ArchangelDesign I have been unable to reproduce this via a unit test thus far, could you share how you are configuring the session?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The isset() is still necessary, and should be performed before the other checks.

@weierophinney
Copy link
Member

@mwillbanks When writing a test, set the container key in the storage to a scalar, truthy value before attempting to retrieve the container. I think that will display the issue.

@mwillbanks
Copy link
Contributor

@weierophinney it will, the question is how is it getting to this state...

@ArchangelDesign are you manually managing the flash messenger's keys at any certain state? The failure should indeed happen here and as @weierophinney stated it is getting set to a scalar of some kind. In this case the container should actually fail as there is already data present of which it should not automatically remove.

I've attempted to look through just about all places of the session code and it does not seem to provide any clues as to what is happening. What I can say is that there might be something where you manually are attempting to manage or clean up a session variable by not leveraging the container but rather setting it to null.

@mwillbanks mwillbanks removed this from the 2.5.2 milestone Jul 29, 2015
@ArchangelDesign
Copy link
Author

sorry for the delay, I will test this solution tomorrow at work, however I don't think it will resolve this issue. I do not set messenger's keys manually (although I've tried that figuring out what went wrong) and I do not use session container at any point before in the application. Changing namespace doesn't help either, however session container works perfectly by itself, which points to messenger issue.

Test case:
in any action of controller call $this->flashMessenger()->addMessage("Success");

the above throws exception of type mismatch.
I've also tried to instantiate the class manually and manually create session container, but method getMessagesFromContainer() still throws an exception due to isset() returning true.

I understand that isset() is required due to the warning issue, but that will leave us with choice to either use customized ZF2 in our project (which I don't like) or to write custom messenger (which my boss doesn't like) :)

There is no rush as far as I'm concerned, the fix I applied works fine for now. I should probably mention that this particular application works as a Moodle 2.7 block, thus there might be some overlap, however there has never been an issue with session container before, nor memory manager or cache.

I will continue investigation tomorrow and let you know what I found.

@mwillbanks
Copy link
Contributor

If you can make a reproduceable test case we can get it fixed, my main concern is finding the source of the problem and fixing that vs. fixing the symptom.

@mwillbanks
Copy link
Contributor

Closing the issue for now; if you find a reproducible test case feel free to open.

@mwillbanks mwillbanks closed this Sep 12, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants