php mysql_num_rows() errorhandling

davewood

Wenn ich einen neuen User in der Datenbank anlegen will, dann frage ich zuvor nach wieviele User mit diesem Namen bereits existieren.

PHP

$query = "SELECT username FROM fm5_user WHERE username='$username'";
$result = mysql_query($query);
if (mysql_num_rows($result) > 0) {
	echo "Username already exists.<br />";
} else {
	$query =  "INSERT INTO fm5_user VALUES('$username', '$email', '$icq', '$tel', '$password', NOW())";
	echo $query . "<br />";
	if (mysql_query($query)) {
		echo "You are now a FM5'ler.<br />";
	} else {
		echo "Something went wrong. :) " . mysql_error() . "<br />";
	}
}

Alles anzeigen

Das Problem ist nur das dieser Ausdruck

PHP

if (mysql_num_rows($result) > 0) {

auch dann true ergibt wenn ein Fehler passiert (Ich hab zB zu Testzwecken in der SELECT Query einen tippfehler eingebaut (SELECT * FROM fm5_user WHERE useraname='$username') und der falsche if zweig ausgeführt wird.

Wie gehts richtig?

dankesehr

david

Paulchen

Vielleicht

PHP

if(mysql_error()) {
    /* fehler in der abfrage */
}
else {
    /* ... */
}

Btw. SQL-Injection kennst du eh, oder?

davewood

nö, nicht wirklich.

Hatte aber das Wort schon im Hinterkopf und hätte danach gegoogelt?

Würde mich aber auch nicht wehren wenn du mir ein paar Sätze darüber bzw. was man dagegen tun kann schreibst.

david

beefy

Siehe mysql_query(), dort mußt du den Fehler abfangen, nicht bei mysql_num_rows(). Also z.B.

PHP

if (($res = mysql_query($query) === FALSE) {
  // blah
}


// oder auch...
$res = mysql_query($query) or die("blah");

Hab das jetzt nicht getestet (gibt PHP den Wert einer Zuweisung eh wieder als Wert zurück?), aber grundsätzlich geht das so.

Oder, wie ich grad sehe, so wie Paulchen es gepostet hat. Meine Lösung ist aber eleganter.

Paulchen hat recht was das Thema SQL Injection betrifft - schau dir das mal lieber an. Du solltest statt

PHP

$query = "SELECT username FROM fm5_user WHERE username='$username'";

auf jeden Fall

PHP

$query = "SELECT username FROM fm5_user WHERE username='" . mysql_real_escape_string($username) . "'";

benutzen.

Paulchen

Ja, sonst kommt der böse Paul und gibt als Benutzernamen

Code

blabla'; DROP TABLE fm5_user WHERE 1=1 OR username='bla

ein und dann kommt als Abfrage

SQL

SELECT username FROM fm5_user WHERE username='blabla'; DELETE FROM fm5_user WHERE 1=1 OR username='bla'

heraus. :devil:

beefy

Du hättest halt grad noch Glück weil sich MySQL eh weigert, mehrere Befehle innerhalb einer Query zu akzeptieren. Dem Paulchen würd aber sicher noch was noch teuflischeres einfallen.

bosie

nimm lieber gespeicherte prozeduren, dann gehste bzgl sql injection auf nummer sicher

Paulchen

Zitat von bosie

nimm lieber gespeicherte prozeduren

Was angesichts der Tatsache, dass es die erst ab MySQL 5 gibt, leider nicht immer möglich ist.

bosie

Zitat von Paulchen

Was angesichts der Tatsache, dass es die erst ab MySQL 5 gibt, leider nicht immer möglich ist.

hm, wo wird erwähnt, dass er nicht mysql 5 hat?

Paulchen

Zitat von bosie

hm, wo wird erwähnt, dass er nicht mysql 5 hat?

Meine Aussage hat sich nicht konkret auf den vorliegenden Fall bezogen.

davewood

hatte vor allem kein php5 aber habs nun lokal installiert (unet.univie rennt nur php 4)

Andre Frage (SQL Injections wären gelöst)

PHP

// escape special characters to prevent SQL Injection
	public function quote_smart($query) {
		// Quote if not a number or a numeric string
		if (!is_numeric($query)) {
			$query = "'" . mysql_real_escape_string($query, $this->db) . "'";
		}
		return $query;
	}

Ich scheitere ein wenig an folgendem:
Wenn ein User zum ersten mal auf die Seite kommt, dann wird ne Session gestartet.
Ich würde gern ein Objekt DatabaseController verwenden welches den Zugriff auf die MySQL Datenbank kapselt.
Dieses soll ein Singleton werden, also einmal erzeugt und dann immer nur die Instanz geholt, aber irgendwie funktioniert das nicht so wie ich will.

PHP

class DatabaseController {
	private $host="xxx";
	private $user="xxx";
	private $password="xxx";
	private $db_name="xxx";
	private $db;
	private static $dbc;

	private function connect() {
		$this->db = @mysql_connect($this->host, $this->user, $this->password) 
			or die('Could not connect to MySQL Database: ' . mysql_error());
		@mysql_select_db($this->db_name, $this->db) 
			or die('Could not select MySQL Database: ' . mysql_error());
	}

	// execute query and return array with values
	public function exec_query($query) {
		$result = mysql_query($query, $this->db)
			or die('MySQL ERROR: ' . mysql_error());
		if($result === TRUE) {
			// INSERT, ... Operations return a boolean
			// just return the result of that operation
			return $result;
		} else {
			$i = 0;
			$ret = array();
			while($row = mysql_fetch_array($result, MYSQL_ASSOC)) {
				$ret[$i] = $row;
				$i++;
			}
		}
		// Free resultset
		mysql_free_result($result);
		return $ret;
	}

	protected function __construct() {
		$this->connect();
	}


	public static function instance() {
		if (!is_object($dbc)) {
			$dbc = new DatabaseController();
		}
		return $dbc;
	}


	public function __destruct() {
		unset($this->rights);
	}
}

Alles anzeigen

und im Code mach ich ein include auf das file und dann zB

PHP

include_once("DatabaseController.php");
$dbc = DatabaseController::instance();

beefy

Zitat von davewood
PHP
// escape special characters to prevent SQL Injection
	public function quote_smart($query) {
		// Quote if not a number or a numeric string
		if (!is_numeric($query)) {
			$query = "'" . mysql_real_escape_string($query, $this->db) . "'";
		}
		return $query;
	}
Es tut dir nicht weh, auch Zahlen zu escapen und unter Anführungszeichen an MySQL zu schicken. Ich würde das im Zweifelsfall einfach immer machen, dann kann man auch nicht darauf vergessen.

Siehe dazu z.B. auch den Code aus einem Kommentar im PHP-Manual, scheint mir recht praktisch zu sein:
PHP
<?php


// example call:
// $results = db_query("SELECT * FROM users WHERE username='?' AND password = PASSWORD('?');", $username,$password);


function db_query($query) {
  $args  = func_get_args();
  $query = array_shift($args);
  $query = str_replace("?", "%s", $query);
  $args  = array_map('mysql_real_escape_string', $args);
  array_unshift($args,$query);
  $query = call_user_func_array('sprintf',$args);
  $result = mysql_query($query) or die('Query failed: ' . mysql_error());
  return $result;
}


?>
Alles anzeigen
[QUOTE=davewood]Dieses soll ein Singleton werden, also einmal erzeugt und dann immer nur die Instanz geholt, aber irgendwie funktioniert das nicht so wie ich will.

Der Code scheint auf den allerersten Blick in Ordnung zu sein, was genau geht denn nicht?

davewood

hum.

das objekt wird bei jedem seiteaufruf (also in meinem fall dann wenn ich einen ajax request schicke) neu instantiiert, die if abfrage in

PHP

public static function instance() {
   if (!is_object($dbc)) {
      $dbc = new DatabaseController();
   }
   return $dbc;
}

ergibt scheinbar immer true (also dass es sich um kein objekt handelt)

hab grad keinen php5 server verfügbar um das nochmals zu testen.

david

Jensi

Zitat von beefy

Es tut dir nicht weh, auch Zahlen zu escapen und unter Anführungszeichen an MySQL zu schicken.

Ich benutze dafür gerne is_numeric(). Dann weiß ich nämlich auch gleich, ob die Eingabe der Erwartung entspricht, bevor die Query vielleicht schiefgeht; und wenn is_numeric() == true, dann ist auch escapen unnötig. Z. B. einfach

Code

if (!is_numeric($userid))
        exit false;

php mysql_num_rows() errorhandling

Jetzt mitmachen!

Benutzer online in diesem Thema