Question

Je travaille sur un assez grand projet dans lequel il y a beaucoup d'endroits où le code comme le suivant existe:

function foo($a, $b, $c, $d, $e, $f) {
    $clean = array();
    $mysql = array();

    $clean['a'] = htmlentities($a);
    $clean['b'] = htmlentities($b);
    $clean['c'] = htmlentities($c);
    $clean['d'] = htmlentities($d);
    //...

    $mysql['a'] = mysql_real_escape_string($clean['a']);
    $mysql['b'] = mysql_real_escape_string($clean['b']);
    $mysql['c'] = mysql_real_escape_string($clean['c']);
    $mysql['d'] = mysql_real_escape_string($clean['d']);
    //...

    //construct and execute an SQL query using the data in $mysql
    $query = "INSERT INTO a_table
              SET a='{$mysql['a']}',
                  b='{$mysql['b']}',
                  c='{$mysql['c']}',
                  d='{$mysql['d']}'";
}

De toute évidence, cela jette de nombreux avertissements en PHP pour les index non définis.

Est-il vraiment nécessaire de réécrire le code comme suit?

function foo($a, $b, $c, $d, $e, $f) {
    $clean = array();
    $mysql = array();

    $clean['a'] = htmlentities($a);
    $clean['b'] = htmlentities($b);
    $clean['c'] = htmlentities($c);
    $clean['d'] = htmlentities($d);
    //...

    $mysql['a'] = (isset($clean['a'])) ? mysql_real_escape_string($clean['a']) : mysql_real_escape_string($a);
    $mysql['b'] = (isset($clean['b'])) ? mysql_real_escape_string($clean['b']) : mysql_real_escape_string($b);
    $mysql['c'] = (isset($clean['c'])) ? mysql_real_escape_string($clean['c']) : mysql_real_escape_string($c);
    $mysql['d'] = (isset($clean['d'])) ? mysql_real_escape_string($clean['d']) : mysql_real_escape_string($d);
    //...

    //construct and execute an SQL query using the data in $mysql
    if (isset($mysql['a']) and isset($mysql['b']) and isset($mysql['c']) and isset($mysql['d'])) {
        $query = "INSERT INTO a_table
                  SET a='{$mysql['a']}',
                      b='{$mysql['b']}',
                      c='{$mysql['c']}',
                      d='{$mysql['d']}'";
    }

}
Était-ce utile?

La solution

Vous pouvez beaucoup simplifier votre fonction si vous utilisez:

function foo($a, $b, $c, $d, $e, $f) {

    $args = func_get_args();   // or build an array() manually

    $args = array_map("htmlentities", $args);
    $args = array_map("mysql_real_escape_string", $args);

    list($a, $b, $c, $d, $e, $f) = $args;

Le contrôle Isset () à la position montrée semble complètement inutile. Les variables étaient déjà définies.

Autres conseils

Est-il nécessaire d'avoir une telle fonction codée dure?

J'utilise ceci:

function insert_array($table, $data) {  
    $cols = '(';
    $values = '(';
    foreach ($data as $key=>$value) { 
        $value = mysql_real_escape_string($value);
        $cols .= "$key,";  
        $values .= "'$value',";  
    }
    $cols = rtrim($cols, ',').')';
    $values = rtrim($values, ',').')';  
    $sql = "INSERT INTO $table $cols VALUES $values";
    mysql_query($sql) or die(mysql_error());
}

Ensuite, pour insérer des données quel que soit son nom et ses colonnes à utiliser:

$data = array('id' => 1, 'name' => 'Bob', 'url' => 'foo.com');
insert_array('users', $data);

Oui, si l'indice de tableau ou la variable n'existe pas PHP, donnez un avertissement / avis.

La bonne façon est de vérifier chaque variable avant de les utiliser avec isset() fonction.

C'est une bonne pratique de les vérifier avant de l'utiliser.

Vous devez vérifier les index qui pourraient ou non exister. Cependant, votre code est assez déroutant et votre véritable code semble probablement totalement différent. Dans cet exemple de code, les clés existent évidemment, vous venez de les créer vous-même.

  1. Dans votre exemple, vous pouvez déplacer la partie mysql_real_escape_string à l'intérieur du si vous vérifiez les variables, alors vous savez déjà qu'elles existent.

  2. Il n'y a aucune raison d'utiliser des tableaux ici, vous pouvez les stocker dans la même variable.

  3. XSS-Protection (htmlTentities (), notez que cela ne suffit pas) doit être fait avant les données de détraquage, pas avant de stocker comme dans votre exemple. Une des raisons est que vous vous retrouverez avec des trucs codés / échappés plusieurs fois. Le HTML / JS malveillant ne peut pas nuire dans votre base de données.

Si votre projet est très important, les index non définis peuvent devenir un cauchemar de la ligne, surtout s'ils contiennent des données générées par l'entrée de l'utilisateur, et en particulier en l'absence de bons rapports d'erreur avec le traçage de la pile. En effet, à mesure que les données sont remises entre les demandes, rien ne garantit qu'il a été défini au point d'entrée d'origine, et donc vous finissez par faire beaucoup de vérifications redondantes pour les valeurs nulles ou vides.

Vous voudrez peut-être examiner si ce que vous essayez d'accomplir ici pourrait ne pas être mieux réalisé en faisant de cette fonctionnalité un objet. Les valeurs que vous représentez avec $ a $ b et $ C pourraient facilement être transformées en propriétés d'objet et une méthode SAVE () pourrait économiser l'état dans la base de données.

En dehors de cela, vous pouvez effectuer votre chèque beaucoup plus rapidement et beaucoup plus de manière cohérente en utilisant une boucle foreach. Il suffit de parcourir les données par ses clés et d'effectuer la véritable évasion et les HTMLentités dans le corps de la boucle.

http://php.net/manual/en/control-structures.foreach.php

Je suggère également de HTMLPurifier pour votre filtrage XSS, car très souvent HTMLentites est insuffisant, en particulier pour les formulaires publics qui acceptent que le contenu utilisateur soit placé dans l'application Web.

http://htmlpurifier.org/

Licencié sous: CC-BY-SA avec attribution
Non affilié à StackOverflow
scroll top