Basic coding question about PHP syntax

[busboy]

Is it better to do something like this at the top a PHP page:

if (isset($_SESSION['firstName'])) {
	
	$firstName = $_SESSION['firstName'];
}

if (isset($_SESSION['sponsorID'])) {
	
	$sponsorID = $_SESSION['sponsorID'];
}

And then be able to successfully evaluate the values like this:

if ($firstName && $sponsorNumber > 0) {
   //Execute code
}

I am asking this because I’m trying to clean up my PHP across my website, paying attention to the E-Notices that Apache is showing me in the error log. One of them was “Undefined Index”, so I solved it by using isset in my code. However, once I started doing this, my IF statements were failing. Here is an example:

if (isset($_SESSION['firstName']) && isset($_SESSION['sponsorNumber']) > 0) {
   //Execute code
}

Thank your for your help and patience.

[oddz]

if(isset($_SESSION['firstName'],$_SESSION['sponsorNumber']) && $_SESSION['sponsorNumber'] > 0) {
}

What the code currently does is check if firstName key exists and if the RETURN value of isset against $_SESSION[‘sponsorNumber’] is 0. Which actually means that when sponsorNumber exists it can be anything because the value returned by isset will be true (bool) which will casted to 1.

In general though when dealing with arrays you should always check the existence of an array key. Furthermore, when dealing with user controlled input of input that can be changed by the end user such as; cookie or form data validation is necessary.

[Michael_Morris]

$_SESSION is not user controlled. There’s no reason to copy $_SESSION variables to local variables - it’s busy-box coding and only creates an additional failure point in the code, particularly during refactoring.

[felgall]

@Michael_Morris

$_SESSION is not user controlled. There’s no reason to copy $_SESSION variables to local variables

There is a possibility of users tampering with $_SESSION data - so you should consider sanitiising it and moving the sanitised result to a local field if you are concerned about the possibility of tampering.

If you are not concerned about tampering then you should definitely NOT copy it as that could compromise the security of your entire application as you will not know which fields are sanitised and which are not.

[Michael_Morris]

How? The data never leaves the server so unless your code saved a tainted value its impossible.

[John_Betong]

@busboy

I am asking this because I’m trying to clean up my PHP across my website, paying attention to the E-Notices that Apache is showing me in the error log. One of them was “Undefined Index”, so I solved it by using isset in my code.

Try these setting at the top of your page to try and trap your errors locally before updating to your server:

  defined('LOCALHOST') ?: define('LOCALHOST', 'localhost'===$_SERVER['SERVER_NAME']);
  if(LOCALHOST) {
    // SHOULD BOTH BE SET IN php.ini
    error_reporting(-1);  
    ini_set('display_errors', true);
  }

  session_start();
    $_firstName = (isset($_SESSION['firstName'])) ? $_SESSION['firstName'] : NULL; 
    $_sponsorID = (isset($_SESSION['sponsorID'])) ? $_SESSION['sponsorID'] : NULL; 
    $_sponsorNO = (isset($_SESSION['sponsorNO'])) ? $_SESSION['sponsorNO'] : NULL; 
  
  // Validate
    if($_firstName && $_sponsorID && $_sponsorNO) {
      echo '<pre>'; print_r($_SESSION); echo '</pre>';

    }else{
      $_SESSION['firstName'] = '$_firstName';
      $_SESSION['sponsorID'] = '$_sponsorID';
      $_SESSION['sponsorNO'] = '$_sponsorNO';
      echo '<pre>'; print_r($_SESSION); echo '</pre>';
    }    

Has anyone any further Debugging suggestions?