PHP: Error in your SQL syntax

[Flo (Gast)]

Hallo,

ich versuche seit einigen Stunden vergeblich einen leeren Return von 
einer SQL Abfrage abzufangen.

Vereinfacht mache ich folgendes:

<?php
$user_agent = $_SERVER['HTTP_USER_AGENT'];
$connection = mysql_connect($server,$user,$password);
$db = mysql_select_db($database,$connection);
$browser_sql  = "SELECT * FROM `BrowserOS` WHERE browser = '$user_agent'";
$browser_result = mysql_query($browser_sql) OR die(mysql_error());
  
if(mysql_num_rows($browser_result)<1){

$insert_sql = "INSERT INTO BrowserOS (browser) VALUES ($user_agent);";
$insert_result = mysql_query($insert_sql) OR die(mysql_error());
};

Also den Browser des Aufrufenden ermitteln, wenn dieser noch nicht in 
der Tabelle BrowserOS steht, dann eintragen.

Jedoch bekomme ich jedes mal wenn der Browser noch nicht in der Tabelle 
steht, folgende Fehlermeldung bei der Abfrage 
"if(mysql_num_rows($browser_result)<1){" --> You have an error in your 
SQL syntax; check the manual that corresponds to your MySQL server 
version for the right syntax to use near '(Windows NT 6.1; 
WOW64).....1985.' at line 1

Sobald ich die Abfrage auskommentiere und damit diesen Wert in die 
Datenbank schreiben lasse, kommt keine Fehlermeldung.

Also kommt der Fehler nur wenn der Mysql keine Tabellen zurückliefert.


Habt hier noch jemand einen Tip für mich?

[K. J. (Gast)]

so richtig kenne ich mich damit nicht aus aber ich würde auf das ";" in 
der abfrage tippen.

[Peter II (Gast)]

Flo schrieb:
> Also kommt der Fehler nur wenn der Mysql keine Tabellen zurückliefert.

dann liefere doch einfach einen wert zurück:

$browser_sql  = "SELECT Count(*) as Anz FROM `BrowserOS` WHERE browser = 
'$user_agent'";

und dann solltest du dich mir Prepared SQL beschäftigen, denn so hast du 
schon die erste Sicherheitslücke in deinem Programm.

[eingast (Gast)]

Flo schrieb:
> $insert_sql = "INSERT INTO BrowserOS (browser) VALUES ($user_agent);";

versuche es mal damit:

$insert_sql = "INSERT INTO BrowserOS (browser) VALUES ('$user_agent');";

[Flo (Gast)]

eingast schrieb:
> versuche es mal damit:
>
> $insert_sql = "INSERT INTO BrowserOS (browser) VALUES ('$user_agent');";

Der Insert ist nicht das Problem, wenn ich die if Abfrage auskommentiere 
dann trägt er den Wert richtig ein, das Problem liegt nur in der Abfrage 
wenn es noch keinen Eintrag gibt..

Das mit Count ist eine gute Idee, das werde ich ausprobieren

[Rufus Τ. F. (rufus)]

$browser_sql  = "SELECT * FROM `BrowserOS` WHERE browser = '$user_agent'";

Warum hier "Backticks" um BrowserOS und Hochkommata um $user_agent,

$insert_sql = "INSERT INTO BrowserOS (browser) VALUES ($user_agent);";

... wenn sie hier nicht nötig sind?

[Karl H. (kbuchegg)]

Vor allen Dingen fehlt hier ein ; im SQL Statement

$browser_sql  = "SELECT * FROM `BrowserOS` WHERE browser = '$user_agent'";

--->

$browser_sql  = "SELECT * FROM `BrowserOS` WHERE browser = '$user_agent';";

Wie MySQL das handhabt kann ich nicht sagen, aber in den Command Line 
Tools wirkt sich so ein fehlender ; so aus, dass damit das SQL Statement 
noch nicht vollständig ist und mit der nächsten Eingabe weiter geht. 
Wenn das dazu führt, dass damit in einem einzigen SQL Statement sowohl 
ein SELECT als auch ein INSERT vorkommt, dann setzt das einen Fehler.

[Peter II (Gast)]

Rufus Τ. Firefly schrieb:
> Warum hier "Backticks" um BrowserOS und Hochkommata um $user_agent,
> $insert_sql = "INSERT INTO BrowserOS (browser) VALUES ($user_agent);";
>
> ... wenn sie hier nicht nötig sind?

ja sieht merkwürdig aus, ist aber ok.

[Flo (Gast)]

das war dann ein Copy&Paste Fehler von mir... habe mir teilweise die 
Querys aus PHPMyadmin kopiert.

ich habe es jetzt mal mit count versucht, an das Ergebnis (im moment 0 
weil die Tabelle leer ist) komme ich noch nicht ganz:

$browser_result = mysql_query($browser_sql) OR die(mysql_error());
$count = mysql_result($browser_result);
  
if($count == 0){
  $insert_sql = "INSERT INTO BrowserOS (browser) VALUES $user_agent);";
  $insert_result = mysql_query($insert_sql) OR die(mysql_error());
  echo "Wird hinzugefuegt";    
    };

Ich glaube mysql_result($browser_result); ist hier nicht das richtige 
oder?
Es kommt weiterhin:You have an error in your SQL syntax; check the 
manual that corresponds to your MySQL server version for the right 
syntax to use near '(Windows NT 6.1; WOW64)...'

[Peter II (Gast)]

Karl Heinz schrieb:
> Vor allen Dingen fehlt hier ein ; im SQL Statement

nein, auch das ist nicht der Fehler. Das Braucht man um im Client das 
Kommando an den Server zu übergeben. Wenn man die Datenbank über PHP 
anspricht ist es nicht notwendig.

[Flo (Gast)]

vergessen:  $browser_sql  = "SELECT count(*) FROM `BrowserOS` WHERE 
browser = '$user_agent'";

[eingast (Gast)]

Flo schrieb:
> Der Insert ist nicht das Problem, wenn ich die if Abfrage auskommentiere
> dann trägt er den Wert richtig ein, das Problem liegt nur in der Abfrage
> wenn es noch keinen Eintrag gibt..

Würde ich jetzt mal so nicht behaupten, denn die Fehlermeldung deutet 
auf das ";" in der Browserbezeichnng hin, welches als SQL-Kommandoende 
interpretiert wird. Deshalb versuche es erstmal mit den "'" und dann 
kann man weiter diskutieren!

[Läubi .. (laeubi)]

Flo schrieb:
> Ich glaube

Lieber Doku lesen als glauben... Dein Problem ist NICHT irgendwelche 
Funktionsaufrufe oder existierende Tabelle(spalten) sondern das dein 
Query falsch ist. String müssen IMMER mit 
http://php.net/manual/de/function.mysql-real-escape-string.php escaped 
(und in Hochkomma eingeschlossen o.ä.) werden weil sonst teile des 
String (wie er dir ja auch sagt) als SQL Befehl interpretiert werden.

> Also kommt der Fehler nur wenn der Mysql keine Tabellen zurückliefert
Wenn der keine Tabbellen mehr hat ist was anderes kaputt...

> bei der Abfrage "if(mysql_num_rows($browser_result)<1){"
Höchst unwahrscheinlich, da hier ÜBERHAUPT kein SQL ausgeführt wird!

[Flo (Gast)]

eingast schrieb:
> Würde ich jetzt mal so nicht behaupten, denn die Fehlermeldung deutet
> auf das ";" in der Browserbezeichnng hin, welches als SQL-Kommandoende
> interpretiert wird. Deshalb versuche es erstmal mit den "'" und dann
> kann man weiter diskutieren!

SUPER, danke, der Fehler kam vom insert...

Jetzt habe ich es so:

    
$browser_sql  = "SELECT count(*) FROM `BrowserOS` WHERE browser = $user_agent'";
   
$browser_result = mysql_query($browser_sql) OR die(mysql_error());
$count = mysql_result($browser_result);
  
if($count == 0){
  $insert_sql = "INSERT INTO BrowserOS (browser) VALUES    ('$user_agent')";
  $insert_result = mysql_query($insert_sql) OR die(mysql_error());
  echo "Wird hinzugefuegt";    
    };

leider Ignoriert er jetzt das if($count == 0) und macht den Insert bei 
jedem aufruf.

[eingast (Gast)]

Flo schrieb:
> leider Ignoriert er jetzt das if($count == 0) und macht den Insert bei
> jedem aufruf.

das ist ein anderes Thema und dabei wird dir die Doku helfen...

[Läubi .. (laeubi)]

Flo schrieb:
> SUPER, danke, der Fehler kam vom insert...

Ach...

Flo schrieb:
> leider Ignoriert er

Würd' ich auch machen wenn jemand zu faul ist mal strukturiert 
vorzugehen...

[Peter II (Gast)]

Flo schrieb:
> $count = mysql_result($browser_result);
>
> if($count == 0){

ist ja auch Blödsinn.

woher soll er denn wissen welche Spalte du haben willst?

[Peter II (Gast)]

Nachtrag:

du musst doch mindestens erst mal eine Zeilen "fetchen" und dann kann 
man von der Zeile eine Spalte abfragen.

[Karl H. (kbuchegg)]

Schon wieder ein COpy&Paste Fehler, oder fehlt hier tatsächlich ein ' 
vor dem $user_agent?

$browser_sql  = "SELECT count(*) FROM `BrowserOS` WHERE browser = $user_agent'";

[Karl H. (kbuchegg)]

Peter II schrieb:
> Flo schrieb:
>> $count = mysql_result($browser_result);
>>
>> if($count == 0){
>
> ist ja auch Blödsinn.
>
> woher soll er denn wissen welche Spalte du haben willst?

Ich denke, die Idee ist sich zu fragen, wieviele Records überhaupt im 
Ergebnis vorliegen.

[Εrnst B. (ernst)]

Und: mach das unbedingt noch sicher!

Wie oben schon geschrieben: So ist das eine Sicherheitslücke.

Der "User Agent" ist vom Webseiten-Besucher frei vorgebbar,

Der Erste Besucher mit User-Agent

'; drop database; --

Bereitet dir viel Freude.

Und ja: Es gibt Tools um solche Lücken automatisch zu finden und 
auszunutzen.

[Flo (Gast)]

Das Problem lag nur am Insert, ich habe es jetzt wie folgt gelöst:

$browser_sql  = "SELECT * FROM `BrowserOS` WHERE browser = '$user_agent'";
$browser_result = mysql_query($browser_sql) OR die(mysql_error());
    
    if(mysql_num_rows($browser_result)<1){  
    $insert_sql = "INSERT INTO BrowserOS (browser) VALUES  
                ('$user_agent')";
    $insert_result = mysql_query($insert_sql) OR 
                die(mysql_error());
    echo "Wird hinzugefuegt";    
    };

Εrnst B✶ schrieb:
> Und: mach das unbedingt noch sicher!
>
> Wie oben schon geschrieben: So ist das eine Sicherheitslücke.
>
> Der "User Agent" ist vom Webseiten-Besucher frei vorgebbar,

Wäre das eine mögliche Lösung?
$user_agent = str_replace("'","Einbruchsversuch",$user_agent);

[TestX .. (xaos)]

Flo schrieb:
> Wäre das eine mögliche Lösung?
> $user_agent = str_replace("'","Einbruchsversuch",$user_agent);

Nein! Schmeiß deinen SQL Kram komplett weg! Die mysql_ / mysqli_ 
funktionen stammen aus den Anfängen von PHP und werden leider durch 
unzählige Tutorials so weitergegeben...heute benutzt man die Funktionen 
nur noch für eigene DB-Abstraktionsschichten...

Du solltest konkret PDO mit "Prepared Statements" benutzen - siehe 
http://php.net/manual/de/book.pdo.php - das verhindert jegliche Probleme 
dieser Art! Jede Variable wird über "bind(key, value)" dem Statement 
hinzugefügt und automatisch escaped

Dazu gibts auch unmengen an Tutorials im Internet...wenn du dir selber 
einen gefallen tun willst befolge den Rat...

[Εrnst B. (ernst)]

Flo schrieb:
> Wäre das eine mögliche Lösung?
> $user_agent = str_replace("'","Einbruchsversuch",$user_agent);

Nicht wirklich.
etwas besser wäre "addslashes", "mysql_escape_string", oder besser 
"mysql_real_escape_string".

Eine Richtige Lösung sind alle drei auch nicht.
Besser: Ein neueres mysql-Interface nehmen, nicht das steinalte 
"mysql_*"-Zeuchs. Gerade bei Neuentwicklungen gibt es da eigentlich 
garkeinen Grund mehr für.

http://php.net/manual/de/mysqli.quickstart.prepared-statements.php
Oder besser gleich PDO-Mysql.

[Flo (Gast)]

Super, danke für den Tip! Ich habe mir mal eben ein Beispiel gesucht und 
es auf meine Anwendung angepasst, das mit dem schließen ist mir noch 
etwas unklar...

<?php 
$handler = new mysqli("dbhost", "dbuser", "dbpass", "db");

$getuseragent = $handler->prepare("SELECT * FROM people WHERE (user_agent = ?)");
$getuseragent->bind_param('s', $user_agent);
$user_agent = $_SERVER['HTTP_USER_AGENT'];
$getuseragent->execute();
  
if (!$getuseragent->get_result()) {
    echo("Browser nicht vorhanden, wird hinzugefuegt...");
    $insertbrowser = $handler->prepare("INSERT INTO BrowserOS (browser) VALUES (?)");
  $insertbrowser->bind_param("s", $browsername);
  $name = $user_agent;
    $insertbrowser->execute();
  //nicht sicher ob das hier stimmt
  $insertbrowser->close();
    
}; 
  //nicht sicher ob das hier stimmt
$getuseragent->close();
 ?>

[Peter II (Gast)]

Flo schrieb:
> if (!$getuseragent->get_result()) {
>     echo("Browser nicht vorhanden, wird hinzugefuegt...");

das sieht mir komisch aus. Auch ein leeres Result ist auch ein Result. 
Es kommt aus einen Select immer eine Result zurück, eventuell hat es 
aber 0 Zeilen.

[Flo (Gast)]

vermutlich ist es dann so besser:
$getuseragent->execute();
$getuseragent->store_result();

if ($stmt->num_rows<1) {
....

[Läubi .. (laeubi)]

Wenn man die Anzahl haben will sollte man sie auch einfach abrufen...

SELECT count(*) as count FROM people WHERE (user_agent = ?)

Spart dem DB server allen möglichen Quatsch zu übertragen den du eh nie 
auswertest...

[Slin (Gast)]

Falls du noch keine Lösung hast:

http://php.net/manual/de/function.mysql-num-rows.php


<?php

$link = mysql_connect("localhost", "mysql_user", "mysql_password");
mysql_select_db("database", $link);

$result = mysql_query("SELECT * FROM table1", $link);
$num_rows = mysql_num_rows($result);

echo "$num_rows Zeilen\n";

?>

Auf ersten Augenblick schein mir so zu sein (ich hab nicht ausprobiert):
Du hast den Connection ($link) für mysql_query nicht übergeben. Damit 
ist es ein Aufrufproblem (kein SQL Problem an der Stelle), $result war 
undefiniert, und dann bei mysql_num_rows ist es rausgestiegen.

Das ist nur ein Tipp ;)