Dragging an old PHP program into the 21st Century!

[JaycomIT]

Hi All,

I have inherited an old, looks like, bespoke program. From what I can make out it was written circa 2006 and hasn’t been updated since the server it was running on has gone for a long nap and I have moved it to a nice new server from a back up. The server is running php 7.2.20

My PHP is basic and I have gone through the files and changed most mysql to mysqli so now the basic system comes up but I am getting a lot of errors so I am hoping this forum is the place I can come for help and advice?

I am getting errors on the page such as:

Warning : mysqli_query() expects at least 2 parameters, 1 given in /var/www/vhosts/****/*******/sign/index.php on line 51

Warning : mysqli_num_rows() expects parameter 1 to be mysqli_result, null given in /var/www/vhosts//*****/sign/index.php on line 53

Those lines are:
$status = mysqli_query(“SELECT * FROM status WHERE office = ‘$office’”);
/* Work out how many staff we have */
$status_tot = mysqli_num_rows($status);

Have I come to the correct place?

Cheers in advance

Jayce

[chorn]

Did you have a look at the manual?

https://www.php.net/manual/en/mysqli.query.php

[JaycomIT]

Hi,

Yes I have that open all of the time yet it still throws the errors up.

Jayce

[chorn]

then i don’t know whats unclear

Procedural style
mysqli_query ( mysqli $link , string $query [, int $resultmode = MYSQLI_STORE_RESULT ] ) : mixed

link
Procedural style only: A link identifier returned by mysqli_connect() or mysqli_init()

[phdr]

I have gone through the files and changed most mysql to mysqli so now the basic system comes up

All that did was eliminate the fatal run-time errors about the nonexistent mysql_ statements.The current code is not how to call the mysqli statements. You will need to go through the code at least one more time to fix all the mysqli_query() statement calls.

However, due to the lack of security (putting external/unknown values directly into the sql query statement allows sql injection) and the lack of error handling (the 2nd error message is a follow-on error because the 1st one failed and there’s no error handling), just getting the code to run isn’t enough. You will need to go through the code again to add security and error handling.

Using the PDO extension instead of the mysqli extension, using a prepared query when supplying external/unknown data to the sql query, and using exceptions to handle errors will result in the simplest and safest code. The PDO extension will let you use the result from both a non-prepared and a prepared query in the same way (the mysqli extension has two completely different sets of statements) and in a similar way to how the old mysql_ extension fetched the data. A prepared query, while adding only one statement per query, provides fool-proof protection against sql injection. Using exceptions for errors, which only takes one added statement when you make the database connection, will mean that you don’t have to add error handling logic at each statement that can fail.

Because the old mysql_ extension broke program scope (the last connection was globally available), when converting old code, you might as well use a user written function that does the same thing. This will let you write a query function, that uses the PDO extension internally, to call when executing each query. If this function accepts an optional 2nd call-time parameter that consists of an array of input parameters, you can write it to use a non-prepared query when there are no inputs, and use a prepared query when there are inputs. You can do a search/replace in the code to substitute this function for all the original mysql_query() calls. You would then need to go through each query that has external/unknown data in it, replace any variables with ? place-holders, and supply the removed variable(s) as an array as the 2nd call-time parameter. This function will return a PDOStatement object, which can be used in the rest of the code in a similar way as the old mysql_ result resource. By writing a few more user written functions, you can replace the original mysql num_rows and fetch statements with functions that use the equivalent PDO statements internally.

[JaycomIT]

Thanks for that reply, that will be the best course of action long term but short term I need to get this into a useable state as all of the job cards etc are on here and they don’t have the months it would take me to re-write the whole thing

[phdr]

I didn’t state to rewrite the whole thing. I stated a simple and secure method that will result in the least amount of changes to the existing code.Using the above suggestions, the two lines of code you posted would look like -

$status = pdo_query("SELECT * FROM status WHERE office = ?",[$office]);
$status_tot = pdo_num_rows($status);

[JaycomIT]

apologies, just been told my brother has throat cancer so I wasn’t thinking straight for a bit

I put those two lines in and got this instead?

Fatal error : Uncaught Error: Call to undefined function pdo_query() in /var/www/vhosts//***/sign/index.php:51 Stack trace: #0 {main} thrown in /var/www/vhosts///sign/index.php on line 51

[phdr]

Using the above suggestions

Until you read, understand, and actually do the things that I wrote in that reply, you won’t be able to get the code to work.

[JaycomIT]

I think I’ll leave it till tomorrow, my mind isn’t on things at the moment

[phdr]

Typical PDO connection code. Put this into a separate .php file and require it when needed -

$DB_HOST = ''; // db host name or ip address
$DB_USER = ''; // db username
$DB_PASS = ''; // db password
$DB_NAME = ''; // database name
$DB_ENCODING = 'utf8'; // db character encoding

$pdo = new pdo("mysql:host=$DB_HOST;dbname=$DB_NAME;charset=$DB_ENCODING",$DB_USER,$DB_PASS);
$pdo->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION); // set the error mode to exceptions
$pdo->setAttribute(PDO::ATTR_EMULATE_PREPARES,false); // run real prepared queries
$pdo->setAttribute(PDO::ATTR_DEFAULT_FETCH_MODE,PDO::FETCH_ASSOC); // set default fetch mode to assoc

Code for the query function -

// note: if the original code is supplying the old mysql connection link in any call, remove it.
function pdo_query($sql, $params = null)
{
	global $pdo; // since the mysql_ extension broke program scope, mimic that too.
	
	if(empty($params)){
		// no inputs
		$stmt = $pdo->query($sql);
	} else {
		// has inputs
		$stmt = $pdo->prepare($sql);
		$stmt->execute($params);
	}
	return $stmt;
}

Code for the num_rows function -

function pdo_num_rows($stmt)
{
	// by definition this is being used with a MySQL database and the rowCount() function will work for all query types
	return $stmt->rowCount();
}

[JaycomIT]

Hi, Sorry but I’m back again, unfortunately I’ve had to have some time off to help my brother with his cancer ordeal but it looks like he’s on the right track now

I have added the code above but I must be missing something, like I said my PHP is beginner level but once the above code is in I get the following error Parse error : syntax error, unexpected ‘status’ (T_STRING), expecting ‘)’ in /var/www/vhosts//time..co.uk/sign/index.php** on line 51 surely there must be somewhere I need to put the sql string in?

[chorn]

it does not look like the error comes from the code you posted, or you may have changed something since then, so i would recommend you take a fresh copy from your script and post line 51 and some more lines before and after that.

[JaycomIT]

Hi,

Thanks for the reply, line 51 is:

/* Get the list for that office */

function pdo_query($sql, $params = SELECT * FROM status WHERE office = ‘$office’”)
{
global $pdo; // since the mysql_ extension broke program scope, mimic that too.

[chorn]

at least you are missing some quotation marks.

And put your code into code-tags, you have this button in the editor: </>

[phdr]

User written functions have two parts - 1) the function definition, where the function code (prototype) is defined, and 2) the function call, where you actually make use of the function in your code.

The following is an example of calling a function -

$status = pdo_query("SELECT * FROM status WHERE office = ?",[$office]);
$status_tot = pdo_num_rows($status);

The following is an example of a function definition -

// note: if the original code is supplying the old mysql connection link in any call, remove it.
function pdo_query($sql, $params = null)
{
	global $pdo; // since the mysql_ extension broke program scope, mimic that too.
	
	if(empty($params)){
		// no inputs
		$stmt = $pdo->query($sql);
	} else {
		// has inputs
		$stmt = $pdo->prepare($sql);
		$stmt->execute($params);
	}
	return $stmt;
}

[JaycomIT]

Right I see (I think!) those changes have now cleared that error and we’ve now moved on to an uncaught error on line 63

} else {
// has inputs
$stmt = $pdo->prepare($sql);
$stmt->execute($params);
}

[chorn]

to an uncaught error on line 63

so why don’t you tell us what error message your get?

[JaycomIT]

Sorry, trying to do too many things at once, forgot to paste it in

Fatal error : Uncaught Error: Call to a member function prepare() on null in ***: pdo_query(‘SELECT * FROM s…’, Array) #1 {main} thrown in /var/www/vhosts/*/*/sign/index.php on line 63

[phdr]

That would mean that $pdo is a null, which would mean that you never made a database connection. Code to make the database connection using the PDO extension has been posted above in this thread.