How to not write PHP

I know, I know, who am I say?

Well, anyway, I came across the following link tonight:
http://pastebin.ca/592702

Here is a quick list of what is wrong:

  1. There is absolutely no control-flow, an attacker can do anything he wants and controls your page with simple GET.
  2. Values from GPC are not quoted properly in SQL queries.
  3. Instead of clearly seperating PHP code (backend) and HTML (frontend) all is messedmixed up.
  4. Use of the dreaded shorttag - I wish PHP would get rid off that soon!
If you write code like this, you just feed the Java trolls and the general mis-understanding about the quality of PHP applications. Also, if you sell your services to other people, please make sure you cannot be held responsible for what you do because you would be in big trouble if you are.

Tags:
This also appears in:

Next Posts

Previous Posts

Comments

Marquis de Morieve
Hm, was hast du denn gegen das short tag ? Ich find's toll..

Mein Code sieht leider nicht viel besser aussieht als dein Beispiel, wie man es nicht machen sollte :p

Schick mir mal nen Beispiel, wie man es richtig macht ;)
till

Hm, was hast du denn gegen das short tag ? Ich find's toll..

Mein Code sieht leider nicht viel besser aussieht als dein Beispiel, wie man es nicht machen sollte :p

Schick mir mal nen Beispiel, wie man es richtig macht ;)


Wenn Du das shorttag abschaltest, dann kannst du wieder "<?xml" schreiben, ohne es in ein echo zu kapseln.

Wie man es meiner Meinung nach richtiger macht, hab ich ja beschrieben oder angedeutet. Im Bezug auf quoting in SQL-Abfragen, waere das einfachste quoting), addslashes(stripslashes($foo)) zu machen, obwohl es natuerlich auch Funktionen wie mysql_real_escape_string gibt.

Generell waere es auch besser, wenn man mit Typen arbeitet. Denn obwohl alles in PHP ganz schnell ein String ist, gibt es zum Beispiel auch noch viele Andere - ganze Zahlen, Dezimalzahlen, Objekte, Listen usw..

Die SQL-Abfrage wird zum Beispiel auch schneller wenn Du Dich am Typ orientierst.

$id = (int) @$_GET['id'];
if (empty($id)) {
// redirect or whatever
}
$query = "SELECT foo FROM bar WHERE id = " . $id;
// etc.

Oder bei String:
$user = @$_GET['user'];
if (empty($user)) {
// do something else
}
$query = "SELECT foo FROM bar WHERE user = '%s'";
$query = sprintf($query, mysql_real_escape_string($user));
// continue

Generell ist es viel uebersichtlicher, wenn man zum Beispiel die Datenbankabfragen vor dem Layout (HTML, usw.) macht, damit man ggf. auch einen Status darstellen kann - "wurde nicht gefunden", "ein Fehler ist aufgetreten" usw..
till
Villeicht noch eine Anmerkung, mit dem @ bin ich natuerlich auch sehr faul.

Viel besser ist:

if (isset($_GET['id'])) {
// yes, we have ;-)
}
Marquis de Morieve
Naja, daß per isset geprüft wird, ob's die Variable gibt, mache ich auch, und Fehlermeldungen lege ich als variablenstring in der Datenbank config Datei ab.
Datenbankzugriffe per function..
Aber so recht übersichtlich ist das bei mir trotzdem nicht, und typen benutze ich auch nicht =)

till
Ist zwar auch schon fast ein Schimpfwort, aber schau Dir doch mal ein paar MVC-Frameworks an. Die zwingen Dich bis zu einem gewissen Punkt ordentlicher zu werden. ;-)

Wenn Du PHP5 hast, dann wuerde ich Dir das Zend Framework oder Symfony empfehlen. Bei PHP4 am ehesten CakePHP.

Post a comment

Already a Vox member? Sign in

You will be asked to join Vox to post this comment.