Upgrading PHP to current

[AdmiralQ]

Let me start by saying ... I suck.

I am a high school Spanish teacher who manages his own WHM, cPanel, and site. I use PHP extensively on my site to create tools for my students. Nothing any one of you would call advanced coding, but I can do some really cool stuff with some really basic PHP and MySQL dB access. All that being said, let's go back to the first thing I said ... I suck. And here's why. I have not upgraded my PHP since version 5.4, and I have long since disabled automatic upgrades. My website is somewhere around 1,100 files which I've been building for the past 11 years and I have a great fear of upgrades to PHP and the truncating of old PHP rendering my site disabled. Imagine the task of going through 1,100 files many of which are several hundred lines of code long and re-coding truncated old PHP code, and then debugging the new code. I LOVE programming in PHP but I fear I'm about to reach a point where because of the age of my code that it won't work anymore. I have built the last ten years of my career as an adaptive and improvisational teacher on this code and website. So, my question is how real is my fear that if I upgrade to PHP 8 that I will encounter problems? You see my code is so old that I'm not even using the new MySQL protocols like "mysqli_query()." I still use "mysql_query()." I understand that there are reasons to start using the new format. I've just got a website full of code whose format is 11 years out of date.

[mac_gyver]

there are migration appendices in the php documentation that list the removed and backward incompatible changes in each php version.

the mysql_ database extension was removed, so, that code won't run at all and will need to be updated. if your code is organized so that the database specific code is grouped together and located before the start of the html document on any page, updating the database 'layer' would be straightforward. you just need to produce the same query result that the rest of the code needs. if your code is not organized like this, doing so will make it easier to test and debug the database code. if you do update any of the database specific code, forget about the mysqli extension. it is overly complicated and inconsistent when dealing with non-prepared and prepared queries, requiring you to essentially learn two different sets of statements. instead, learn and use the much simpler and better designed PDO extension.

instead of putting dynamic values directly into sql query statements, you should use prepared queries. if code is currently using any ...escape_string() or addslashes() functions, these are removed. converting an old query to a prepared one is straightforward - 

1. remove, and keep for later, any php variables that are inside the sql query statement. note: any wild-card characters in a LIKE comparison are supplied as part of the data value
2. remove any quotes or {} around the value and any concatenation dots/extra quotes that were used to get the php variable into the sql query statement
3. put a simple ? prepared query place-holder into the sql query statement for each value
4. call the PDO prepare() method for the sql query statement
5. call the PDOStatement execute() method with an array of the variables you removed in step #1
6. for a query that returns a result set, fetch the data from the query. see the fetch() method when fetching a single row of data. the fetchAll() method when fetching all the rows of data at once. and occasionally the fetchColum() method when fetching a single column from a single row of data. forget about any num_rows method or property. just fetch then test if/how many rows of data there are.

php8+ uses exceptions by default for database statements that can fail - connection, query, exec, prepare, and execute. when using exceptions no discrete error checking logic will get executed upon an error and should be removed. your main code will only see error-free execution. if execution continues past a statement that can throw an exception, you know there was no error, without needing any program logic. the only database exceptions you should catch and handle in your code are for user recoverable errors, such as when inserting/updating duplicate user submitted data. the exception catch block would test for a duplicate index error number and setup a message for the user letting them know what was wrong with the data that they submitted. for all other error numbers, just rethrow the exception and let php handle it and for all other type of queries do nothing in your code and let php catch and handle any database exception, where php will use its error related settings to control what happens with the actual error information, via an uncaught exception error (database errors will get displayed or logged the same as php errors.)

two other major things that old code likely used that have been removed are register_globals and magic_quotes. for register_globals, you will need to use the correct super-global variable that data is in - $_POST, $_FILES, $_GET, $_COOKIE, and $_SESSION. for magic_quotes, any conditional logic testing for them and applying stripslashes() can be removed.

old code found on the web is also filled with unnecessary things, such as trying to strip tags from data, copying variables to other variables for nothing, and missed out on applying htmlentities() to dynamic values being output in a html context, right before outputting it.

if you want to post examples of your existing code, i/we can show what it would look like using current practices.

Edited August 3 by mac_gyver

[Barand]

You have already left it too long, but the longer you put it off the worse it will get.

There are definite advantages to upgrading with new features having been added (both in PHP 8 and MySQL 8). The downside is your database processing will need to change. I'd also strongly recommend you use PDO rather than switching to mysqli (There is more involved than just adding an "I" to the function calls).

How much extra coding will need changing will depend on how you coded in the past. For example, PHP8 is stricter on variable typing. EG

<?php
$a = '';
$b = 5;
echo $a + $b;

You probably get away with that at the moment , the '' being silently converted to '0', but with v8 you get

PHP Fatal error:  Uncaught TypeError: Unsupported operand types: string + int in C:\inetpub\wwwroot\test.php:4

Note that $a = '0' will work as it is convertible to int 0.

[mac_gyver]

if/when you update your code, here is a list of practices, based on a review of the login code in your other thread, that will help make the code more secure, provide a better user experience, eliminate unnecessary code, and though having usable validation logic and error handling will get your code to tell you when and why it doesn't work -

1. always use a full opening php tag - <?php
2. don't use $_REQUEST. use the correct $_GET, $_POST, or $_COOKIE variable that you expect data to be in
3. don't use extract() as this allows hackers to inject their data values into your code, which is what register_globals allowed, which is why they were removed from php
4. use 'require' for things your code must have
5. post method form processing code needs to detect if a post method form was submitted before referencing any of the form data
6. you need to trim, mainly so that you can detect if a value was all white-space characters, then validate all input data before using it
7. you should list out the columns you are SELECTing in a query
8. as already written, you would use prepared queries
9. you need to hash the stored passwords. see php's password_hash() and password_verify()
10. as already written, you would use exceptions for database error handling, and remove any existing error handling logic
11. you would fetch and test if there is fetched data, meaning that the username was matched, then verify the password hash
12. the only user related data that you should store in a session variable upon successful authentication is the user id (autoincrement primary index). you would query on each page request to get any other user data, permissions, ... this will insure that any changes made to this other user data will take effect on the very next page request, without requiring the user to log out and back in again
13. you should only create and store data in the session variable(s) if authentication was successful. by unconditionally creating it/them, you now have session variables that exist, even if they are empty/null
14. the first redirect implies that the form and form processing code are on separate pages. they should be on the same page. the code for any page should be laid out in this general order - 1. initialization, 2. post method form processing, 3. get method business logic - get/produce data needed to display the page, 4. html document
15. by redirecting around on your site and using values from the url to control what the form page reports, your site is open to phishing attacks, where someone could trick your users to enter their username/password on the phishing site, then redirect them to your site and make it look like they just entered an incorrect username/password
16. the only redirect you should have in your code is upon successful completion of post method form processing and it should be to the exact same URL of the current page to cause a get request for that page. this will prevent the browser from trying to resubmit the form data should that page get browsed back to or reloaded
17. if you want to allow the user to go to a different page, provide navigation links, or more simply, just integrate the login operation on any page that needs it
18. every redirect needs an exit/die statement to stop php code execution. while you have the remainder of the code inside an else {...} conditional branch, if you had stopped the php code after the redirect, you could just put the remainder of the code w/o the else
19. the inno_login_track_staff INSERT query is part of the successful authentication code. after the logic is changed to use password_verify() and only store data to session variable(s) upon successful authentication, there will be a specific place in the php logic to put this query
20. inno_school_staff - this indicates you have separate 'user' tables for students and staff. you should have one user/authentication table. the user data that you query for on each page request would control what the current user can see or do on any page
21. don't copy variables to other variables for nothing and don't use intermediate variables when you only reference a value once
22. as an advanced programming practice, if you have more than 2-3 form fields, you should dynamically validate and process the form data, and dynamically produce the form, rather than writing out bespoke code for every field every time you do something different

 

[gizmola]

You should have at least upgraded to php 5.6 previously.  I assume you didn't do that because you you have used the old mysql_ functions.  There is actually a polyfill library (that even the author disavows use of) but point in fact, will work.

So at bare minimum, if you updated to the last available 5.x release you could use it to get around having to rewrite all of your database code.  It also will work with higher level versions of php:  https://github.com/dshafik/php7-mysql-shim

There are tools (static analysis) that can be used to analyze your code and tell you what the issues are and suggest fixes.  Some of these tools will integrate into an IDE/Editor.  The defacto standard IDE for PHP is PHPStorm, but you can also use Visual Studio Code with a plugin like inteliphense, and look up instructions.  Since you are in an educational environment you probably are eligible for a very low cost phpstorm license (as are your students).  

Here's a link to one of the current well known static analysis tools for php:  https://phpstan.org/

Another popular tool that people will also configure into their IDE is:  https://cs.symfony.com/

[gizmola]

One other thing, I wanted to say, is that you should immediately and without hesitation, go through your code and change every instance of the short open tag:

 

<?

and change that to:

<?php

This is a simple search and replace, so long as you can identify whitespace with whatever tool you are using for the search.  You want to find <?(whitespace) and just change the <? to <?php while leaving the whitespace alone.  A regex is great for this, using capturing groups.  Once you figure it out, you should be able to fix your entire codebase in the matter of a few minutes.

The short open tag was made non-default some 15 years or more ago.  The problem here is that any upgrade that potentially modifies or replaces the php.ini or move to a new environment or version of php potentially breaks your site, and there's just no good reason to subject yourself to that possibility.

It's also best practice to Not use the php end tag, unless you need it for intermixture of html and php code.

A simpler rule of thumb is that no .php file should ever end with the php end tag, if that's the last thing in the file (other than perhaps whitespace).

These standards are codified in https://www.php-fig.org/psr/psr-1/ and https://www.php-fig.org/psr/psr-12/