We help IT Professionals succeed at work.

PHP Insert statment incorrect values

dsg138 asked
I have some code where my users are required to choose a team.  There are 32 different buttons,, one for each team.
If the player chose team #5, the value "teamid" would be 5 and the following would be inserted into the DB:
-  UserID = 100 //for example
-  XMLID = l.mlb.com-t.5
-  WeekNum = 1  //if it were week #1

This would be a correct entry.  Most entries get successfully inserted.

Occasionally, I get the following added to my DB which is Incorrect and causes issues:
-  UserID = 100 //for example
-  XMLID = l.mlb.com-t.
-  WeekNum = 1  //if it were week #1
If this is added to my DB, it causes an issue since the user thinks their Team 5 went through, but it really didn't.

I'm not sure how this is possible since each of the buttons contains a different teamid.

Is there a way I can modify my code below so that the incorrect values never get inserted into my php database?

Code is below.  

if( isset($_POST["teamid"]) ){

    $finalteam = "l.mlb.com-t." . $_POST["teamid"];
<div class="notification success">
        <div class="text">
        	<p><strong>Success!</strong> Your selection has been saved! </p>

$PickID = $a4[XMLID];

	UserID = '$MemberID',
	XMLID = '$finalteam',
	WeekNum = '$PickWeek'"
$sql = mysql_query($sql);


Open in new window

Again, the above scenario is rare, but it's been happening now about 1% of the time.  I'm trying to isolate it and it possibly could be due to a mobile browser.  Any suggestions on how I can modify my statement?
Watch Question

Principal Data Engineer
It looks like your form is sending an empty string for teamid in some cases. I'm not sure on why but you should change your php code on line of to this.

if( isset($_POST["teamid"]) && !empty($_POST["teamid"]) {

It sounds a bit strange to me that in most cases it works as expected and in others it does not. Have you checked the output of every button to make sure they all send the correct value?  I would go into your javascript and before the ajax call to the php script do a console.log('whatever the value is or the url');

You can hit F12 while your browser is open and click on the console tab if you are in chrome. That will log the actual value that is being sent in from every button.  If you are not using ajax to make the call and you really should, then use var_dump($_POST);

and put that in your php script one the first line then on the 2nd line kill the script with die();

Start clicking each button and check what var_dumps spits out. You will see if any are blank then you can trouble shoot the button and fix the bad one.
Most Valuable Expert 2011
Top Expert 2016
I see a few things that may help.  First, you really, really want to learn about how to use external data safely in a query string.  This is dangerous:

$finalteam = "l.mlb.com-t." . $_POST["teamid"];

The reason there is danger is that $finalteam is used directly in the query string, and it's not filtered or escaped, so anything (or nothing) in that value can be injected into your query.  The security mantra must always be "filter input, escape output."

Second, you want PHP to tell you if you're doing things wrong.  PHP is silent about many error conditions unless your script specifically asks for insight.  If you add error_reporting(E_ALL) to the top of all of your PHP scripts you will be able to see a notice when your script accidentally relies on an undefined variable (that may be what is happening here).

Third, MySQL is not a black box.  It can and will fail for reasons that are outside of your control.  If your programming does not detect these failures and recover or raise an error message, you will never know what happened until your data base is damaged.  Add to that the fact that PHP is doing away with MySQL support (it's already deprecated) and you've got a bit of work to do just to keep your scripts running, at all, in the future.  Fortunately there is not a huge amount of work, and all of it (including correct error handling) is documented in this article.

Going forward, you may want to adopt a programming design pattern that provides a filter method for every expected external input.  If any of the filters fails or does not run, discard the request.  As an example, if your script expects to receive an integer between 1 and 32, test the input values to see if the input is an integer, and if it is greater than zero and less than 33.  Only if all of the filter method tests pass can your script use the external input.  All other inputs must be given default values, or the requests must be discarded.  If you program it right, your script never relies directly on $_POST -- it only uses data that has been taken from $_POST and passed through your filters.


Thanks guys.  This is really helpful.

I added your validation so that the SQL never runs if teamid is empty.
if( isset($_POST["teamid"]) && !empty($_POST["teamid"]) ){

That makes a lot of sense.  By not filtering my inputs, I was taking a risk on inserting whatever is generated by my users.  Your article makes a great case to rewrite some of my older scripts that use the mysql extensions.

Thanks guys, appreciate the help!


Thanks guys.  Great expert advice!
Mark BradyPrincipal Data Engineer

You're most welcome! I'm sure you will learn a whole bunch of really good practices in the article Ray sent you. There is nothing like learning the correct and safest way to do things when you are dealing with databases. Remember one thing, injections are very easy to do inside one of your form controls. It will modify the actual query that php does and can ruin your day. Sanitize ALL data that comes from an external source like a user input in a form and values sent in a GET request. Can't be too careful!  Good luck