In the spirit of Common programming mistakes for .NET developers to avoid?, what are common mistakes PHP developers make?
Using == when === should be used is a common one. What are some others (preferably from your own experience)?
Community wiki'ed for complete satisfaction.
Putting everything in one file
This is really easy to do, but after a while you realize that the file is too complicated and will easily break. If you can, split the file into several files, with functions in each file. The best method is to go object oriented (I assume you are using PHP5, its 2009 for gods sake!) and create classes with functions. This might slow down your script a bit, and will make a lot more data, but everything gets separated into nice objects.
I'd say the most common error in php is not checking user-input (and as it is a language which is mostly used for the web, there's almost always some user-input somewhere), making way for (My)SQL-Injections or XSS. I've even seen people passing user-input directly to eval()...
Data Access must be done right. By "right" I mean proper encapsulation of data-access logic, no SQL concatenation and please-please do use parametrized queries: this will help make world a better place.
SQL-injection vulnerabilities due to incorrect usage of mysql_query()
This might be a controversial opinion, but I belive that it's a mistake to use the old family of mysql functions. These are the family of functions prefixed with mysql_. There isn't really anything wrong with them as long as you use them correctly, but unfortunately I've found that most of the time they just aren't used correctly. The end result is that we have sites all over the internet vulnerable to SQL injection.
A better choice is to use an API that supports prepared statements, which solves this problem completely. MySQLi is such a family of functions, and for security purposes it's fine. However, I believe that the API is a bit of a pain to use. The main basis for that opinion is the fact that you simply cannot retrieve the result of a prepared statement as an array.
The best option is to use PDO. It's a modern database agnostic wrapper that supports flexible prepared statements and results in modern, safe and readable code. It's a joy to work with.
We should all stop teaching beginners the old functions and show them how to use and become comfortable with PDO from the start.
Not turning on strict and reporting all errors, and you miss out on typos mistakes.
And there's one in PHP4 where foreach return value, not reference.
Some PHP bugs due to form processing are actually due to bad form HTML.
And here's my nemesis. Using of headers. They are not function calls! I had see code like this:
if ($bUserValid)
{
header("Location: login-success.php");
}
header("Location: login-error.php");
The last header sent is the last header to be executed. So no matter what happens, you will always be redirected to login-error.php
I don't use PHP very often, so the "forget the $ variable prefix" trap often gets me.
Writing code without using the maximum level of error reporting
All development should be done with:
error_reporting(E_ALL | E_STRICT);
All notices and warning should be eliminated during development. It makes the code incredibly much more stable. (Of course all errors should then be hidden on production setups.)
Using $var inside a class when what I really mean is self::$var or $this->var. With warnings off, this doesn't raise any flags.
Unsafe usage of include
You often see things like:
include("pages/" . $_GET["pg"]);
It's a mistake of incredible proportions to not check the pg variable here.
I've got a list for this one from an old blog post I wrote awhile ago.
Unquoted array indexes:
//This is *WRONG* (but will work):
echo $array[my_key];
//This is correct
echo $array['my_key'];
PHP considers the unquoted index as a "bare" string, and considers it a defined constant. When it can't find a matching symbol for this constant in the symbol table however, it converts it to a real string, which is why your code will work. Quoting the index prevents PHP from having to check for the defined constant, and makes it safer in case someone defines a future constant with the same name. I've also heard that it is up to seven times faster than referencing an unquoted index for this reason.
While it's fortunately disappearing from more recent PHP versions, one thing I always hated was how PHP tried to do the best of any developer mistake instead of throwing an error. One such example is that trying to get an undefined constant would return the constant name as a string instead, and unknowing people would "abuse" that:
$user = $_POST[user]; // $_POST['user'], unless user is a defined constant
Using mysql_escape_string() instead of mysql_real_escape_string().
Use single quotes instead of double quotes for strings when possible.
When you surround a PHP string in double quotes, it is subsequently parsed by the PHP interpreter for variables and special characters, such as "\n". If you just want to output a basic string, use single quotes! There is a marginal performance benefit, since the string does not get parsed. If you have variables or special characters, then by all means use double-quotes, but pick single quotes when possible.
echo 'Do this when possible.';
echo "Not this.";
Forgetting to learn the standard library, first.
Much wheel-reinvention is caused by this! (This applies to any programming environment, really)
I personally enjoy writing in PHP. The fun stops when I have to do code reviews. Here's why:
error_reporting(E_ALL | E_STRICT); in development.define is your friend. Please don't make me hunt for silly literals in your code.register_shutdown_function, die / exit is usually a very bad and ungraceful way of handling errors, especially in production. Even still, you're probably making it more complex than it needs to be.if ( condition ) { return true; } else { return false; } or $variable = condition ? true : false; (this applies to any language)."-" == (int)"-" is true.Maybe more, that's it for now.
Edit:
$_REQUEST out of laziness instead of the appropriate $_GET and $_POST.I think a very typical mistake, not even for a beginner is to have some kind of chars before the tag
<?php
Which makes the script thow an error I you want to modify the header. This is also the case if you end you php scripts with a php-tag an then these files get included/required. Nothing complicated, but sometimes hard to find.
This is more of a best practice, but it's definitely the answer to much frustration for new developers - omit the closing tag ("?>") for PHP Files. It is not required by PHP, and omitting it prevents trailing whitespace from being accidentally injected into the output.
Aware of uninitialized arrays
foreach($customers as $customer) {
$customerIds[] = $customer->getId();
}
Always do
$customerIds = Array();
Before loop, it saves you a lot of debugging time.
Having all the page functions return a string, which is concatenated and then printed from a massive if-then-if-else-else-if..... block, rather than splitting it up properly and dropping out of PHP when possible (Portable Hypertext Preprocessor.)
Not escaping the output of any variable that was brought over get POST or GET using htmlspecialchars(). This is to prevent XSS.
This one
var_dump(0123 == 123);
outputs false because 0123 in the comparison is translated to octal because of the leading zero, while
var_dump("0123" == 123);
outputs true because "0123" is converted to integer 123
Never compare floats for inequality.
var_dump(0.7 + 0.1 == 0.8);
outputs false. No kidding.
This is due to the fact that it is impossible to express some fractions in decimal notation with a finite number of digits. For instance, 1/3 in decimal form becomes 0.3.
If higher precision is necessary, the arbitrary precision math functions and gmp functions are available.
Source: PHP: Floating point numbers check out the warning part
Incorrect testing of strpos / stripos
if (strpos('Needle in a haystack', 'Needle')) {
echo "There's a needle in the haystack!";
} else {
echo "There's no needle...";
}
This will output "There's no needle...", as strpos will return 0, which is interpreted as false.
Should be:
if (strpos('Needle in a haystack', 'Needle') !== false) {
Googling for every answer
One of the first things everyone should learn is to read the documentation, it's not that hard. The docs are excellent and answer almost every question.
I heard from several newbies independently that the docs are hard to understand, apparently the function signatures look too confusing. I half blame this on the dynamically typed nature of PHP, which makes people not think about variable types and gets them confused when confronted with something like:
string chunk_split ( string $body [, int $chunklen [, string $end ]] )
When all they are used to see is:
chunk_split('my text');
Forgetting to add break; at the end of each case for your switch statement (assuming, of course, that you did not intend to execute case after case).
I often find that the biggest mistake with PHP is worrying too much about the difference between single and double quotes, or the difference between print and echo when you should be worring about the loop within a loop that's actually taking up most of the processing time!
PHP coders are experts as the incredible detail of a tip or trick (how many discussions have we seen about ++i instead of i++, while at the same time, they're counting the length of the array in each loop, which takes more time than ++i and i++ put together!)
PHP references != C pointers
With PHP reference passing, what f() is passing to g() is a handle that will let g() change the assignment of one of f()'s local variables. This sounds almost the same as a C pointer, but in C, g() doesn't change the assignment of f's local variable, it changes the contents of the memory location it points to.
I found this quite disturbing when I got it for the first time, firstly because my local variables were a lot less local than I thought they were; and secondly I had trouble with the very idea of functions having access to each others' local variable scopes, which I wasn't familiar with from other languages I'd used... what languages do have that feature?
Another C habit which doesn't apply in PHP is passing pointers around to avoid memory being duplicated. As far as I understand, this isn't necessary, as the PHP interpreter uses copy-on-write tricks to avoid reassigning memory until it has to.
There was a very good article on this, maybe in the PHP Architect magazine, with boxes and arrows explaining step-by-step what was going on in a program execution. If somebody can remember where that is and can find a URL for it, can they append it here?
Saving as UTF-8 (with BOM)
PHP will not throw any warnings or errors but you will have strange "markup" objects (appearing as white lines in the DOM tree inside Firebug) on the page. If you construct the page using multiple templates they appear at the places where your template starts. Random margins and whatnot will haunt your dreams...
Spaces before <?php
If you have spaces before any <php tag in one of your files, you will not be able to send headers or set cookies. You will however get a decent warning if you have the correct error display settings on your development system.
Use a Framework. There are a lot of very good and excelent php application frameworks out there, to name a few YII, PRADO, LIMONADE, SOLAR, CakePHP, Symfony, CodeIgnite .... [PLACE YOURS HERE] ... do not reinvent the wheel (again ... and again ... and again).
Do your homework and identify which one better suits your requirements/likes and go start using it. It will save you tons of time.
Calling a method just because method_exists() returned true. Note that just because a method exists does not mean it is callable. method_exists returns true even if the method is protected or private.
Copying online examples like:
$res = mysql_query( "select from persons where name = $name and password = $password" );
Or things line:
$output = "<b>$name</b>";
Or even:
$email = "From: <$from>\n"
. "To: <$to>\n"
. "Subject: $subject"
Basically, everywhere you combine strings from multiple sources, the variables need to be escaped. That includes:
and so on.
Otherwise, You can easily login with strings like 'password' or 1 --, inject HTML to read user login cookies, or add new SMTP/HTTP headers by adding a newline character in the input..
As solution, there is:
mysql_real_escape_string()
htmlentities()
urlencode()
preg_quote()
json_encode()
escapeshellarg()
Ideally, for database queries I strongly suggest to avoid the mysql_...() functions directly, and use - or build - a wrapper library around it. Something which offers an API line:
$rs = $db->select( "select from persons where name = ? and password = ?", array( $name, $password ) )
foreach( $rs as $record ) // $rs is an object which implements the Iterator interface
{
...
}
If you use a reference in a foreach loop, then you need to unset it afterwards.
Ie.
foreach($results as &$result)
{
...
}
unset($result);