was ist sicherer/besser ?

[jmc]

Falls du noch im Vorhinein eine Prüfung der variabel durchführen mächtest wäre folgender Code sinnvoll:

function sqlstr(&$addr, $def = ""){ // $addr : referencevariable , $def : default value
 if(isset($addr)){
  $addr = mysql_real_escape_string($addr); // reference = querystring for later use and return value
 }else{
  $addr = $def; // reference = default value and return value
 }
 return $addr; // return reference
}
$eintrag = "INSERT INTO tabelle (id, beta, zeit) 
VALUES ('', '" .  sqlstr($_POST['beta']) . "', '" . time() . "')"; 
// reference and $_POST['beta'] correlate and is available for later use
// time() makes perfectly sense here because datebase server and php server may differ
$eintragen = mysql_query($eintrag)or die(__LINE__ .':'. mysql_error()); 
// why first setting a variable for one single use?
if($_POST['beta'] === ""){ // $_POST['beta'] can be used now without hesitation because setted for sure due to sqlstr
 // make that and make this...
} 

[PHP-Desaster]

Das reicht so nicht, jmc. Du solltest nicht einfach eine leere Query an die DB senden, sondern lieber vorher prüfen, ob der POST-Wert gesetzt ist. Ist dies nicht der Fall, kannst du davon ausgehen, dass dein Formular nicht/nicht richtig versendet wurde und in diesem Falle solltest du lieber eine entsprechende Fehlermeldung ausgeben.
Zu dem id-Feld für den nächsten auto_increment-Wert und dem time() habe ich ja schon was gesagt!

Btw: Brich deinen Code doch bitte mal um!

[jmc]

tut mir Leid, dass der Code nicht umgebrochen war. Genau damit kannst du ja die Existenz der POST- Variable testen. Ich beschäftige mich jetzt seit 3 jahren intensiv mit Assembler und wenn du dir einmal den hiermit erstellten Code ansiehst kannst du erkennen, dass er sowohl kürzer, als auch schneller ist, falls auf den an sqlstr übergebenen Wert mindestens einmal im Nachhinein noch mit mysql_real_escape_string zugegriffen wird, als wenn du z.B. sowas machst:

if(isset($_POST['bla']){
 mysql_real_escape_string($_POST['bla']);
}else{
 //...
} 

[skys]

hallo,

wollte doch noch fragen, ob dies beides gleich sicher/gut ist :

$eintrag = "INSERT INTO tabelle (id, beta, zeit)
VALUES('', '" .  sqlstr($_POST['beta']) . "', '" . time() . "')";
$eintragen = mysql_query($eintrag)or 
die(__LINE__ .':'. mysql_error()); 

und

mysql_query("INSERT INTO tabelle (`id` , `beta` , `zeit`)
VALUES ('', '" .  $_POST['beta'] . "', '" . time() . "')"); 

Nein, da du bei der zweiten Abfrage kein Escaping vornimmst!

ups, das escapen habe ich vergessen, stimmt.

ich meinte vor allem den unterschied zwischen variante a :
[COLOR=blue]$eintrag = "INSERT INTO etc....";
$eintragen = mysql_query($eintrag) etc;[/COLOR]

und dem direkten query, variante b :
[COLOR=blue]mysql_query("INSERT INTO etc ");[/COLOR]

ich hätte evtl getippt, daß variante a besser ist, aber keine ahnung.
(escapen bei beiden vorausgesetzt)

bzgl dem anderen prüfungscode brauch ich noch etwas zeit, komme gerade nicht nach mit dem umschreiben und vergleichen
komme aber gerne und bestimmt demnächst darauf zurück

viele grüße, skys

[PHP-Desaster]

Genau damit kannst du ja die Existenz der POST- Variable testen. Ich beschäftige mich jetzt seit 3 jahren intensiv mit Assembler und wenn du dir einmal den hiermit erstellten Code ansiehst kannst du erkennen, dass er sowohl kürzer, als auch schneller ist, falls auf den an sqlstr übergebenen Wert mindestens einmal im Nachhinein noch mit mysql_real_escape_string zugegriffen wird, als wenn du z.B. sowas machst:

Klar testest du auf Existenz, aber macht es oft wenig Sinn, eine Query abzusetzen, wenn entsprechende POST-Daten nicht vorhanden sind. Nimm mal folgenden Dateiaufbau an:

<?php
   if( isset( $_POST['submit'] ) ) {
      $errors = array();
      if( !isset( $_POST['txt'] ) or trim( $_POST['txt'] )=='' ) {
          $errors[] = 'Bitte das Textfeld ausfüllen!';
      }
      // Meinetwegen auch weitere Felder überprüfen!
      if( count( $errors )>0 ) {
         echo "Es sind Fehler aufgetreten:<br />";
         foreach( $errors as $error ) {
            echo $error."<br />";
         }
      }
      else {
         $query = "insert into foo ( '".mysql_real_escape_string( $_POST['txt'] )."' );"
         mysql_query( $query ) or die( mysql_error() );
         echo 'Vielen Dank und Tschüss!';
      }
   }
   else {
?>
<form action="?" method="post">
    <input type="text" name="txt" />
    <input type="submit" name="submit" value="Senden" />
</form>
<?php
   }
?>

Hier macht deine Vorgehensweise lange nicht so viel Sinn!

[H2O]

@jmc

• Das mit der Referenz ist in diesem Fall völlig überflüssig, da du den Wert sowieso zurückgibst
• Was sollen die zwei returns nacheinander?
• Wie von PHP-Desaster angesprochen, wird bei dir nicht auf einen leeren Wert geprüft.
• Wenn schon eine Funktion, zur Überprüfung, dann doch besser eien, die auch noch den Datentyp mit einbezieht, für nummerische Werte ist mysql_real_escape_string() eigentlich sinnlos

Wenn man das berücksichtigt, kommt man vielleicht etwa auf so eine Funktion:

function sql_val($wert, $typ = 'string'){
    if (empty($wert)){
        return false;
    }
    switch ($typ){
        case 'string':
            return mysql_real_escape_string($wert);
            break;
        case 'int':
            return intval($wert);
            break;
        case 'float':
            return floatval($wert);
            break;
        default:
            return false;
    }
}

$beta = sql_val($_POST['beta'], 'string');
if ($beta){
    $eintrag = "INSERT INTO tabelle (beta, zeit) 
                VALUES ('$beta', NOW())"; 
    mysql_query($eintrag) || die(mysql_error() . "<br />Query: $sql<br />"); 
} 

PS. Die Zeit wird in im Typ DATETIME gespeichert, und mit der MySQL-Funktion NOW() eingefügt.
Natürlich kann man die Überprüfung noch ausbauen. Ist ja nur ein Anhaltspunkt

[jmc]

@skys

Beide sind gleich sicher, aber die zweite Variante ist etwas besser, weil da kein Speicher für deine Variable freigegeben werden muss.

Original geschrieben von PHP-Desaster
Klar testest du auf Existenz, aber macht es oft wenig Sinn, eine Query abzusetzen, wenn entsprechende POST-Daten nicht vorhanden sind.

Das ist richtig und das habe ich nie bestritten, meines war nur ein Beispiel. Natürlich solltest du je nach Verwendung die Eingaben erst überprüfen und dann erst an die Datenbank senden.
Deine Variante ginge jedoch noch etwas kürzer bei der Verwendung von mehreren solchen abfolgen:

<?
// hier sqlstr als funktion
 if(isset( $_POST['submit'] ) ){
  $errors = array();
  if(sqlstr($_POST['txt']) == ''){
   $errors[] = 'Bitte das Textfeld ausfüllen!';
  }
  // Meinetwegen auch weitere Felder überprüfen!
  if(isset($errors[0])){
   echo "Es sind Fehler aufgetreten:<br />".implode("<br />", $errors);
  }else{
   mysql_query("INSERT INTO foo ('".$_POST['txt']."');") || die(mysql_error());
   echo 'Vielen Dank und Tschüss!';
  }
 }else{
?>
<form action="?" method="post">
    <input type="text" name="txt" />
    <input type="submit" name="submit" value="Senden" />
</form>
<?php
   }
?>

[list=1][*]Das mit der Referenz ist in diesem Fall völlig überflüssig, da du den Wert sowieso zurückgibst[*]Was sollen die zwei returns nacheinander?[*]Wie von PHP-Desaster angesprochen, wird bei dir nicht auf einen leeren Wert geprüft.[*]Wenn schon eine Funktion, zur Überprüfung, dann doch besser eien, die auch noch den Datentyp mit einbezieht, für nummerische Werte ist mysql_real_escape_string() eigentlich sinnlos[/list=1]

[list=1][*]Die Referenz ist nötig um die selbe Variable wieder zu verwenden und um sie mit isset zu prüfen[*]Das ist ein Fehler meinerseits und sollte ein Kommentar sein[*]Es ist nur ein Beispiel und eine Prüfung sollte natürlich vorgenommen werden, was ich nie bestritten habe, sonder nur wie.[*]Das Beispiel sqlstr() ist keine Funktion zur Überprüfung und floatval, etc. machen hier nicht viel Sinn, da die Datentypen beim eintrag in eine MySQL Datenbank über PHP so oder so angepasst werden.[/list=1]

[H2O]

Die Referenz ist nötig um die selbe Variable wieder zu verwenden und um sie mit isset zu prüfen

Das ist Bullshit. Du übergibst die Variable deiner Funktion. Dort "escape'st" du sie und gibst sie dann zurück. Alles weitere geschieht mit dem zurückgegbenen Wert.
Und das die Datenbank den Typ selber überprüft, mag bei einem INSERT noch zutreffen, innerhalb einer WHERE-Clause würde ich mich nicht darauf verlassen.

[jmc]

function sqlstr($addr, $def = ""){ // Beispiel ohne Referenz
 if(isset($addr)){ // hierbei wird nur die Variable $addr auf Funktionsebene geprüft
  $addr = mysql_real_escape_string($addr);
  // die globale Variable oder die variable im benutzten scope wird nicht verändert
//sondern nur die im scope der Funktion
 }else{
  $addr = $def; // das selbe wie oben
 }
 return $addr; // return reference
}
echo sqlstr($_GET['sd']); // Error -> es wird versucht den Index zu übergeben aber er existiert nicht
// auch später steht $_GET['sd'] bei sauberer Programmierweise nicht ohne Prüfung zur Verfügung 

Die Referenz ist also für die Vermeidung von Fehlern und für die Beibehaltung des aktuellen scopes notwendig.

Bei WHERE-Clauses wird ebenfalls nicht zwischen datentypen unterschieden und die WHERE-Clauses werden im Voraus an den zu durchsuchenden Typ angepasst. Where ist ja nicht einmal case sensitive, wenn du den string nicht als binary übergibst

[tontechniker]

// Error -> es wird versucht den Index zu übergeben aber er existiert nicht // auch später steht $_GET['sd'] bei sauberer Programmierweise nicht ohne Prüfung zur Verfügung

Ist aber keine schöne Kapselung. Wesentlich sinnvoller ist es die Werte schon vorher zu prüfen um die direkt auf Fehler reagieren zu können, bzw. Globale Variablen über eine eigenen Klasse abzufragen.

[jmc]

Je nach Fall mag es sinnvoller sein, diese Variante ist aber ziemlich speicherfreundlich und schnell.

[PHP-Desaster]

Je nach Fall mag es sinnvoller sein, diese Variante ist aber ziemlich speicherfreundlich und schnell.

Wenn es um Speicherfreundlichkeit geht, bist du bei PHP an der falschen Adresse. Eine interpretierte Sprache mit Assembler zu vergleichen ist nicht sinnvoll!
Sorry, aber so einen Furz interessiert doch keinen! In "vernünftigen" Projekten würdest du objektorientiert arbeiten, da haust du dir deinen Speicher so voll, dass mein Vorgehen im Gegensatz zu deiner sqlstr-Funktion der Tropfen auf dem heißen Stein wäre. Speicherengpässe wirst du dann kaum bei einer Variablenüberprüfung finden!
Btw: Du solltest - auch wenn MySQL das für dich kann - die richtigen Datentypen verwenden!

*Just my 2 cents*

[skys]

hallo,

ich muß mich nochmal dazwischen klemmen :

es gingen ein paar verständnisfragen unter und ich würde mich freuen, wenn mir noch jemand sagen könnte, ob ich damit richtig oder falsch liege :

kontrollfrage, ob ich es richtig verstanden habe :

if(isset($addr)){return mysql_real_escape_string($addr);} 
return ""; 

könnte man das so übersetzen :
existiert die für sql zurechtgemachte variable addr bzw ist sie nicht leer, dann weiter, sonst leere felder ausgeben ?

und während mysql_real_escape_string die eingaben in einem formular nur für einen sql-query brauchbar machen, ist aber der if-teil hier oben für die sicherheit wichtig ?

und ersetzt es im prinzip dieses ? :

if ($_POST['hiddenstring'] != '')
{ // wenn die post-variable nicht leer ist
mysql_close($connect);
header("Location: eintragen.php");
} else
{ // die post-variable ist leer - also formular ausgeben
...formular...
} 

und dann noch 2 punkte :
muß ich abfragen auch escapen (gemeint sich solche abfragen, die ohne formularfelder stattfinden) ?
(im php-manual steht sowas unter dem letzten farbigen code-beispiel (vor den kommentaren) bei mysql_real_escape_string)

jmc schrieb dann auch noch folgendes :
"Falls du noch im Vorhinein eine Prüfung der variabel durchführen mächtest..."
nun bin ich mir nicht sicher, ob das euer diskussionspunkt war ?
und ich weiß nicht, wie wichtig/sinnvoll es ist.
ist damit zb gemeint, daß ich eingabefelder so absichere, daß zb nur buchstaben oder nur zahlen gesendet werden können (das könnte ich mir evtl noch sinnvoll vorstellen) ?

reallife hatte in meinem thread unter projekthilfe zb diese gute idee (konnte ich zum damaligen zeitpunkt nur nicht einbauen, da ich nur weiße seiten sah), die vielleicht genau das betrifft :

Kleine Anregung: Wenn Du schon ein Eingabefeld für die zu löschende ID in deinem Formular hast, dann solltest Du vor dem Löschen auch prüfen, ob der Datensatz mit der angegeben ID existiert. Ich vermute, dass dein ID ein auto_increment Wert in der DB ist, demnach solltest Du auch prüfen, ob es sich bei der Eingabe um eine Zahl handelt.

if( preg_match("~^\d+$~", $wegid) )
{
  [...]
} 

dh, ich müßte, um alles mit einzubeziehen :
- zuerst escapen wie zb :
[COLOR=blue]$beta = mysql_real_escape_string($_POST['beta']);[/COLOR]
- dann die funktion sqlstr mit if isset :
[COLOR=blue]function sqlstr(&$addr)
{
if(isset($addr)){return mysql_real_escape_string($addr);}
return "";
}[/COLOR]
- vor dem ende dieses if noch das if preg_match von reallife einbauen
[COLOR=blue]if( preg_match("~^d+$~", $wegid) )
{
[...]
}[/COLOR]
- und dann [COLOR=blue]mysql_query("INSERT INTO ...[/COLOR]

so dann alles richtig ?
viele fragen, ich weiß *sorry*

danke + viele grüße, skys

[jahlives]

existiert die für sql zurechtgemachte variable addr bzw ist sie nicht leer, dann weiter, sonst leere felder ausgeben ?

isset prüft ob eine Var existiert, sagt aber nichts über deren Inhalt aus! Eine Var mit einem Leerstring existiert auch ist aber empty

muß ich abfragen auch escapen (gemeint sich solche abfragen, die ohne formularfelder stattfinden) ?

Nö, das musst du imho nur machen wenn du Usereingaben in eine Query einbaust.

Ich vermute, dass dein ID ein auto_increment Wert in der DB ist, demnach solltest Du auch prüfen, ob es sich bei der Eingabe um eine Zahl handelt.

Es würde auch ausreichen, wenn du einen Typecast auf int machen würdest

$wegid= (int) $_GET['wegid'];
//oder
$wegid= intval($_GET['wegid']); 

Gruss

tobi

[PHP-Desaster]

Ich gehe mal nur auf das letzte Codeschnipselchen ein, da das die Fragen beantworten sollte:

// Prüfen, ob Variable gesetzt ist, also durch Formular übermittelt wurde
if( !isset($_POST['beta']) ) {
    // Nicht gesetzt -> Fehler
}
// Werteüberprüfung. Z.B. ob der eingegebene Benutzername nur die Buchstaben A-Z enthält
// Bei Ziffern kannst du in der Regel mit is_numeric prüfen, reicht meist gemeinsam mit intval aus
if( !preg_match( '/[a-z]+/i', $_POST['beta'] ) ) {
    // Wert falsch -> Fehler
}
// Nur Datenbankkommunikation, wenn keine Fehler
mysql_query("INSERT INTO ..." ); 

Hoffe, du kannst damit was anfangen!