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

[HttpFoundation] Why is the default session name hardcoded? #1418

Closed
jasonbouffard opened this issue Jun 23, 2011 · 7 comments · Fixed by #1517
Closed

[HttpFoundation] Why is the default session name hardcoded? #1418

jasonbouffard opened this issue Jun 23, 2011 · 7 comments · Fixed by #1517

Comments

@jasonbouffard
Copy link

https://github.com/symfony/symfony/blob/master/src/Symfony/Component/HttpFoundation/SessionStorage/NativeSessionStorage.php#L46

I feel like we should pull the sesssion name from session_name() for the default session name. Because the rest of the default params for $cookieDefaults are pulled from php configuration.

@hhamon
Copy link
Contributor

hhamon commented Jun 23, 2011

If you have a closer look at the code, there is an array_merge(). So you can override the session name from the configuration by changing the $options['name'] parameter.

@stloyd
Copy link
Contributor

stloyd commented Jun 24, 2011

Please checkout session section of full framework config.

@jasonbouffard
Copy link
Author

I'm aware we can set up the session name in the app config. My concern is that Symfony is not taking into consideration what a user may have set up in their php.ini or with php_value in an apache config like it does with all the params from session_get_cookie_params();. As you can see from this pull symfony/symfony-standard#94 we really should be configuring our session at the php level not the Symfony level. Hope that makes more sense where I was coming from on this one. Otherwise I really do appreciate all the work that went into Symfony2, it's awesome.

@stloyd
Copy link
Contributor

stloyd commented Jun 25, 2011

@jpb0104 What do you think about mine proposal for fixing this issue ? The problem now is that if you even set up data in config.yml it will be ignored if option session.name in php.ini is not default one. I can change it but... I'm not sure is this good ;-)
Also skipping session_name() can give some "speed-up" to overall code execution...

@stof
Copy link
Member

stof commented Jun 25, 2011

@stloyd If the user set a value explicitly in the configuration, it should be used.

@stloyd
Copy link
Contributor

stloyd commented Jun 28, 2011

@jpb0104 Please check PR #1463.

@jasonbouffard
Copy link
Author

Yep. PR #1463 looks good to me. At least the changes to NativeSessionStorage.php that are related to this bug.
We can close this issue if PR #1463 gets merged or hear otherwise.

stloyd added a commit to stloyd/symfony that referenced this issue Jul 4, 2011
@fabpot fabpot closed this as completed in 88bee2b Jul 4, 2011
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants