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 uses of session_start($_GET['id']) #19

Closed
MasterOdin opened this issue Nov 12, 2017 · 2 comments
Closed

Fix uses of session_start($_GET['id']) #19

MasterOdin opened this issue Nov 12, 2017 · 2 comments
Labels
BUG Code cleanup Priority 2 These are normal bugs that don't break site functionality, but don't necessarily work well.

Comments

@MasterOdin
Copy link
Member

MasterOdin commented Nov 12, 2017

session_start pre 7.0 didn't accept any parameters and post 7.0 accepts an array to override session INI directives. It looks like the code is trying to set the session ID to use if psased one via the $_GET superglobal ($_GET['session_id']), which you'd accomplish via the session_id function instead which does accept a parameter that allows you to set an ID to use (so long as you call it before session_start).

You would therefore most likely want to change all instances of:

session_start($_GET['session_id']);

to be:

session_id($_GET['session_id']);
session_start();

The same thing applies for the places that use the $_POST superglobal as well.

@lramos15
Copy link
Member

My only worry with this is that currently the session_start(); is just being ignored because its throwing a warning and its just proceeding, so currently the code doesn't do what is expected and I'm afraid changing it to be syntactically correct will drastically change how the sessions start to get handle in PHP and mess something up. We use PHP sessions to verify every single user interaction with the server so messing them up would effectively break the site. I am aware that it is syntactically incorrect and some of our new session code follows the session_id($_GET['session_id'); format.

@MasterOdin
Copy link
Member Author

MasterOdin commented Nov 12, 2017

Well you could rip out all instances of session_id and maintain current functionality since the parameter is totally ignored at the moment anyway, and therefore it's sort of pointless to be passing it around.

@lramos15 lramos15 added Priority 2 These are normal bugs that don't break site functionality, but don't necessarily work well. BUG Code cleanup labels Sep 6, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
BUG Code cleanup Priority 2 These are normal bugs that don't break site functionality, but don't necessarily work well.
Projects
None yet
Development

No branches or pull requests

2 participants