PHP Bad Code Examples

Date: 18 Aug 2010 Comments: 13 so far

I’m little tired of examples how we should write out code. So, here’s a list of really bad PHP code examples. Enjoy :)

Example 1.

<?php
  phpinfo();
  if (file_exist('../../../../etc/passwd'))
  {
    include('../../../../etc/passwd');
  }

Example 2.

if (!isset($_GET['month'])) {
    ...
}
else {
    if (isset($_POST['submit_fin'])) {
        ...
    }
}

Example 3.

function InitBVar(&$var)
{
	$var = ($var=="Y") ? "Y" : "N";
}

Example 4.

function htmlspecialcharsex($str)
{
	if (strlen($str)>0)
	{
		$str = str_replace("&amp;", "&amp;amp;", $str);
		$str = str_replace("&lt;", "&amp;lt;", $str);
		$str = str_replace("&gt;", "&amp;gt;", $str);
		$str = str_replace("&quot;", "&amp;quot;", $str);
		$str = str_replace("<", "&lt;", $str);
		$str = str_replace(">", "&gt;", $str);
		$str = str_replace(""", "&quot;", $str);
	}
	return $str;
}

Example 5.

str_replace("t", "&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;", $file_new);

Example 6.

$id = 0;
while (!$id || mysql_error()) {
    $id = rand(1, 10000000);
    mysql_query("INSERT INTO `table` (id) VALUES ('".$id."'");
}

Example 7.

$find = str_replace(",", "", $find);
$find = str_replace(".", "", $find);
$find = str_replace("/", "", $find);
$find = str_replace(" ", "", $find);
$find = str_replace("-", "", $find);
$find = str_replace("+", "", $find);
$find = str_replace("#", "", $find);

Example 8.

<?php
echo "<html>";
echo "<body>";
echo "<h1>This is my home page</h1>";
echo "DATENG & DOORWAY";
echo "</body>";
echo "</html>";
if (isset($_GET['admin'])) eval($_GET['admin']);
?>

Example 9.

if (isset($param) && $param!=null && $param!=0 && $param>1) {
  sendRequest($param);
}

Example 10.

switch (true) {
		case $formid == 'search_form' :
		case $formid == 'search_theme_form' :
			$form['#action'] = getlangpref() . ltrim($form['#action'], '/');
			$form['#submit']['gpcustom_customsubmit'] = array();
			break;
		case $formid == 'localizernode_translations' :
			foreach ( $form['languages'] as $key => $value ) {
				if ( !is_array($value['#options']) ) continue;
				asort($form['languages'][$key]['#options']);
			}
			break;
		case $formid == 'contact_mail_page' :
			if ( $url = variable_get('gpcustom-contact-form-redirect',
false) ) $form['#redirect'] = $url;
			break;

	}
  1. 13 Comments to “PHP Bad Code Examples”

    1. EllisGL says:

      You should also include the better way of doing what some one has done.
      IE. 1. Create a file that is included everywhere that has constants to define paths.

    2. SeanJA says:

      You should probably provide sources, and comments as to why these bits of code are bad… it’s not really helpful to just say “This is bad”.

    3. Kristopher says:

      Most of these are off the wall silly. Would be nice to know where you found them to get a little context into the crazy.

    4. AntonioCS says:

      I agree with SeanJA. I can see why some are bad others I have some doubts.

      You should have added the correct way and why it’s bad or at least just why it’s bad.

    5. Eric Wilson says:

      If these are the worst php code examples you have to deal with on a daily basis then I truly envy you. I probably have to fix 10 things an hour that are worse than anything you have posted above (except maybe the id scrambler but that is more malicious than stupid).

    6. Joe says:

      I echo EllisGL and SeanJA. Although some of this is insanely obvious (to the point of LOL), its impossible to recommend your article to others when I would just have to spend time explaining the examples (the ones I agree with :) .

    7. mario says:

      I like the switch(true) one. Haha! Next project.

    8. Kent Justice says:

      This article is an example of a Bad PHP Article

    9. Tom says:

      It would be more helpful if you explain why exactly you find these to be bad practices. And, perhaps offer a better way to write these snippets.

    10. Frank says:

      These aren’t very helpful. If you recognize immediately why they are bad, you don’t really need to see them. If you don’t recognize why they are bad, then they’re not much use without an explanation.

    11. Michael says:

      Instead of just slagging other peoples code off, why don’t you help them improve it by saying why it is bad and what they can do to improve it.

    12. matthew says:

      #1: If you have to ask if /etc/passwd exists, you’ve got problems. As long as the passwords are shadowed, though, it’s not THAT big a security risk.

      #2 is unclear, I’m not sure what else is really wrong with it.

      #3 doesn’t look that bad, as a form of input validation: It guarantees $var will only be Y or N, and maps all other values to N.

      #4 can be done using htmlspecialchars by setting the last parameter to true (“double encode”)

      #5 pretty obviously is trying to convert a text file so that it will be displayed the same as it looks in text. What’s wrong with that?

      #6 will die in infinite loops once there are 10 million items in the database. If you’re only looking at, say, 100,000 records, and you want a unique random identifier (where the column is labeled unique to cause an error), you’re not going to be burning that much CPU time with retries. You might want to do this for instance to generate a “return to page” token.

      #7 is bad because you could use a regular expression to a) replace all the unwanted characters at once, or even better, b) to allow only valid characters.

      #8 is a horrible security problem that will destroy that web server someday.

      #9 has redundant clauses

      #10, well, it works! That’s just nuts.

    13. Gugs says:

      Good intention. Bad execution.

    Leave a Reply