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

Fix "session_id" parameter for static session IDs in AgaviSessionStorage #1479

Merged
merged 3 commits into from Sep 22, 2015

Conversation

thomasbachem
Copy link
Contributor

No description provided.

@dzuelke
Copy link
Contributor

dzuelke commented Mar 5, 2014

What problem is this solving? Please provide explanations for PRs and use a few comments in the source code.

@thomasbachem
Copy link
Contributor Author

The "session_id" setting isn't working at all right now, as the check for session_id() === '' will always lead to false if the "session_id" parameter is provided, as session_id($this->getParameter('session_id')); is called just before the check.

@dzuelke dzuelke added the defect label Mar 5, 2014
@dzuelke
Copy link
Contributor

dzuelke commented Mar 5, 2014

There's a lot more broken in that part of the code than just session_id() I think. The whole autostart block never executes IIRC. Is that related?

@thomasbachem
Copy link
Contributor Author

It's just that the auto_start setting is not used anywhere – but the block gets still executed. Strictly speaking it isn't related, but we should strip that setting from the docblock.

@thomasbachem thomasbachem reopened this Sep 21, 2015
@thomasbachem thomasbachem merged commit 3f4f7d3 into agavi:master Sep 22, 2015
thomasbachem added a commit that referenced this pull request Sep 22, 2015
* pr-1479:
  3f4f7d3 Add changelog entry
  14a4c5b Add test for "session_id" SessionStorage parameter
  3d4fff8 Fix "session_id" parameter for static session IDs
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants