patternphpMinor
PHP Session handling class
Viewed 0 times
phphandlingclasssession
Problem
I've written a custom PHP session class for handling sessions across the web app. Please review the code and point out mistakes and suggest better handling techniques.
require_once('config.php');
class Sessions {
protected $sessionID;
public function __construct(){
if( !isset($_SESSION) ){
$this->init_session();
}
//session_start();
//$this->sessionID = session_id();
}
public function init_session(){
session_start();
}
public function set_session_id(){
//$this->start_session();
$this->sessionID = session_id();
}
public function get_session_id(){
return $this->sessionID;
}
public function session_exist( $session_name ){
if( isset($_SESSION[$session_name]) ){
return true;
}
else{
return false;
}
}
public function create_session( $session_name , $is_array = false ){
if( !isset($_SESSION[$session_name]) ){
if( $is_array == true ){
$_SESSION[$session_name] = array();
}
else{
$_SESSION[$session_name] = '';
}
}
}
public function insert_value( $session_name , array $data ){
if( is_array($_SESSION[$session_name]) ){
array_push( $_SESSION[$session_name], $data );
}
}
public function display_session( $session_name ){
echo '';print_r($_SESSION[$session_name]);echo '';
}
public function remove_session( $session_name = '' ){
if( !empty($session_name) ){
unset( $_SESSION[$session_name] );
}
else{
unset($_SESSION);
//session_unset();
//session_destroy();
}
}
public function get_session_data( $session_name ){
return $_SESSION[$session_name];
}
public function set_session_data( $session_name , $data ){
$_SESSION[$session_name] = $data;
}
}Solution
Commented out Code
You should remove code that is commented out. If you think that you might need it in the future, think about using version control.
This is only called in your commented out code. Does the user have to call it manually? If they don't,
if-else and
If you have code like this:
you can rewrite it like this:
Also, when you define a function like this, then use it. Instead of
write
XSS
Using Session for XSS might be a possibility, depending on how your code is used. So in
You should remove code that is commented out. If you think that you might need it in the future, think about using version control.
set_session_id()This is only called in your commented out code. Does the user have to call it manually? If they don't,
get_session_id() will return a wrong result. Maybe rewrite it like this (you don't seem to be using the field $sessionID, so you might as well get rid of it):public function get_session_id(){
return session_id();
}if-else and
session_existIf you have code like this:
if( isset($_SESSION[$session_name]) ){
return true;
}
else{
return false;
}you can rewrite it like this:
return isset($_SESSION[$session_name]);Also, when you define a function like this, then use it. Instead of
if( !isset($_SESSION[$session_name]) ){write
if(!session_exist($session_name]) ){XSS
Using Session for XSS might be a possibility, depending on how your code is used. So in
display_session I would clean the session with htmlentities to prevent XSS attacks.Code Snippets
public function get_session_id(){
return session_id();
}if( isset($_SESSION[$session_name]) ){
return true;
}
else{
return false;
}return isset($_SESSION[$session_name]);if( !isset($_SESSION[$session_name]) ){if(!session_exist($session_name]) ){Context
StackExchange Code Review Q#61066, answer score: 9
Revisions (0)
No revisions yet.