Bonjour à tous et à toutes,

Je développe en PHP depuis quelques temps et j'aimerais avoir vos conseils pour m'améliorer... Je sais pas si j'ai le droit de demander ça, mais j'essaye tout de même. Du fait, déjà j'aurais quelques questions qui arrivent sans cesse car je ne sais pas le meilleur niveau optimisation.

  • Conseillez-vous un site où on utilise des $_GET (www.domaine.com/?p=informations ou alors où les fichiers sont directement exécutés du style www.domaine.com/informations.php ?

  • Préférez-vous include_once/require_once ou include/require ?

  • Les '' à la place des "" dans les écho, les requêtes * à la place de chaque colonnes, les ++$erreur à la place des $erreur++, les , à la place des . dans les concaténations sont ils vraiment plus avantageux en terme de performance ?

  • !isset ou empty ? !empty ou isset ?

Pouvez-vous commentez s'il vous plaît mon code ci-dessous pour me donner des conseils d'améliorations ?

if (isset($_POST'inscription'])){
    $pseudo = htmlspecialchars($_POST'pseudo']);
    $pseudo_min = strtolower($pseudo);
    $password = htmlspecialchars($_POST'password']);
    $password_min = strtolower($password);
    $repeat_password = htmlspecialchars($_POST'repeat_password']);
    $prenom = htmlspecialchars(stripslashes($_POST'prenom']));
    $email = htmlspecialchars(stripslashes($_POST'email']));
    $age = htmlspecialchars(stripslashes($_POST'age']));
    $salt = 'Nln&pw124/n@#Jb3PB';
    $erreur = 0;
    if ($pseudo == NULL || $password == NULL || $repeat_password == NULL || $prenom == NULL || $email == NULL || $age == NULL){
    $message .= '<div class="alert error hideit"><i class="icon-warning-sign"></i><p>Vous n\'avez pas rempli les demandes du formulaire.</p><i class="icon-remove close"></i></div>';
    ++$erreur;
    }
    if ($password_min == '123456' || $password_min == 'azerty' || $password_min == 'azertyuiop' || $password_min == '123456789' || $password_min == '1234567890' || $password_min == $pseudo_min){
    $message .= '<div class="alert error hideit"><i class="icon-warning-sign"></i><p>Le mot de passe indiqué est trop simple, choisissez en un autre.</p><i class="icon-remove close"></i></div>';
    ++$erreur;
    }
    if (strlen($password) < 6){
    $message .= '<div class="alert error hideit"><i class="icon-warning-sign"></i><p>Votre mot de passe doit faire au minimum 6 caractères.</p><i class="icon-remove close"></i></div>';
    ++$erreur;
    }
    if (!filter_var($email, FILTER_VALIDATE_EMAIL)){
    $message .= '<div class="alert error hideit"><i class="icon-warning-sign"></i><p>Le format de votre adresse e-mail n\'est pas correct.</p><i class="icon-remove close"></i></div>';
    ++$erreur;
    }
    if (strlen($pseudo) < 3){
    $message .= '<div class="alert error hideit"><i class="icon-warning-sign"></i><p>Le pseudo doit faire au minimum 3 caractères.</p><i class="icon-remove close"></i></div>';
    ++$erreur;
    }
    if ($password != $repeat_password){
    $message .= '<div class="alert error hideit"><i class="icon-warning-sign"></i><p>Les deux mot de passe entrés ne sont pas identiques.</p><i class="icon-remove close"></i></div>';
    ++$erreur;
    }
    if ($age <= 5){
    $message .= '<div class="alert error hideit"><i class="icon-warning-sign"></i><p>C\'est théoriquement impossible que vous ayez cet âge.</p><i class="icon-remove close"></i></div>';
    ++$erreur;
    }
    $req = $bdd->prepare('SELECT email FROM membres WHERE email = :email');
    $req->execute(array('email' => $email));
    $chk_email = $req->fetch();
    if ($chk_email != '0'){
        $message .= '<div class="alert error hideit"><i class="icon-warning-sign"></i><p>L\'adresse e-mail est déjà utilisée sur un autre compte.</p><i class="icon-remove close"></i></div>';
    ++$erreur;
    }
    $req->closeCursor();

    // Désolé pour ce passage...
    if ($pseudo_min == 'tamere' || $pseudo_min == 'filsdepute' || $pseudo_min == 'filsde' || $pseudo_min == 'default' || $pseudo_min == 'pute' || $pseudo_min == 'nick' || $pseudo_min == 'notch' || $pseudo_min == 'grumm'){
        $message .= '<div class="alert error hideit"><i class="icon-warning-sign"></i><p>Ce pseudo n\'est pas autorisé.</p><i class="icon-remove close"></i></div>';
    ++$erreur;
    }
    $req = $bdd->prepare('SELECT pseudo FROM membres WHERE pseudo = :pseudo');
    $req->execute(array('pseudo' => $pseudo));
    $chk_pseudo = $req->fetch();
    if ($chk_pseudo != '0'){
        $message .= '<div class="alert error hideit"><i class="icon-warning-sign"></i><p>Le pseudo est déjà dans notre base de données.</p><i class="icon-remove close"></i></div>';
        ++$erreur;
    }
    $req->closeCursor();
    if ($erreur == 0){
    $password = sha1($password.$salt);
    $req = $bdd->prepare('INSERT INTO membres (pseudo, password, prenom, email, date, age, ip) VALUES (:pseudo, :password, :prenom, :email, :date, :age, :ip)');
    $req->execute(array(
        'pseudo'    => $pseudo,
        'password'  => $password,
        'prenom'    => $prenom,
        'email' => $email,
        'age' => $age,
        'date' => date('U'),
        'ip' => $_SERVER'REMOTE_ADDR']
    ));
        $req->closeCursor();
    $message = '<div class="alert success"><i class="icon-check"></i><p>Inscription correctement effectuée, vous pouvez maintenant <a href="connexion.php">vous connecter</a></p></div>';
    }
}

Merci pour vos conseils, cordialement,

19 réponses


Bonjour,

  • Conseillez-vous un site où on utilise des $_GET (www.domaine.com/?p=informations ou alors où les fichiers sont directement exécutés du style www.domaine.com/informations.php ? Je vous conseil un MVC, avec un "ROUTER" à chaque page demander nous ne verront pas le *.php.

Les '' à la place des "" dans les écho, les requêtes * à la place de chaque colonnes, les ++$erreur à la place des $erreur++, les , à la place des . dans les concaténations sont ils vraiment plus avantageux en terme de performance ? les guillemet simple & double, il ont aucune performance en plus juste qu'elle que avantage pour une * '*' où "*" conviens parfaitement.

  • Préférez-vous include_once/require_once ou include/require ?
    include_once sont comportement est similaire à include, mais la différence est que si le code a déjà été inclus, il ne le sera pas une seconde fois.
    require_once est identique à require mis à part que PHP vérifie si le fichier a déjà été inclus, et si c'est le cas, ne l'inclut pas une deuxième fois.

  • !isset ou empty ? !empty ou isset ?
    isset : Détermine si une variable est définie et est différente de NULL
    empty : Détermine si une variable est vide
    ! = Différent de..
    Ce sont deux chose différente.

Pour votre code je vais faire cela dans la soirée / journée du 11 / 12.
Cordialement :)

Merci beaucoup, j'attends les critiques du code du coup :) Et pour le MVC, est-ce vraiment utile ? Parce que quand je vois que l'idée crée 3 fichiers pour 1 page c'est énorme tout de même.

Le MVC permet d'organiser le code, et cela et plus simple ^^ surtout quand ont change de design

Des petits conseils simples :

  • Le MVC c'est génial. Mais pour un petit site, c'est inutile de mettre en place une telle architecture. C'est un bon entraînement aux frameworks style Symfony 2 par contre.

  • Tes variables, mets leur des lettres minuscules avant pour te repérer plus simplement, savoir à quoi tu as à faire.
    Exemple ton pseudo, tu peux mettre $sNick, s = string, ou $iAge, i = integer, ça rend vraiment la lecture très simple pour savoir si tu manipules un tableau, un objet, une chaîne etc.

Pour le reste de ton code je trouve ça correct, tu pourrais cependant faire une fonction addError($sMessage, $sErrorMessage) qui ressemblera à :

function addError($sMessage, $sErrorMessage, $iError) {
    $sMessage .= '<div class="alert error hideit"><i class="icon-warning-sign"></i><p>'.$sErrorMessage.'</p><i class="icon-remove close"></i></div>';
    ++$iError;
}

Et que tu pourras mettre à la place de chaque ligne où tu répètes beaucoup de code pour rien =)

Ha, fais attention avec isset quand même, je sais que tout le monde le conseille absolument mais si t'as une valeur dans un tableau qui existe mais qui est null, isset renverra false. C'est chiant quand t'as juste une clé qui renvoi vers une valeur null dont tu te fiches puisque seule la clé t'intéresse, à ce moment là vaut mieux utiliser array_key_exists.

Merci nestecha pour l'idée de la fonction. J'en prends note :)

Au niveau de mes templates, n'ayant pas actuellement de système MVC je n'ai pas vraiment de "vues"; tu veux dire dans le code lorsque j'affiche ? Le problème c'est que par exemple si j'ai le pseudo en net ayant des <font color="red">pseudo</font> par exemple, je devrais faire un htmlspecialchars($_SESSION'pseudo']) à moins de définir la session automatiquement en htmlspecialchars ? Mais ça risque de rester assez chiant à faire si je fais des requêtes en fetch et que je dois utiliser les pseudos... je devrais à chaque fois faire un htmlspecialchars; donc en final est-ce vraiment utile ?

Au niveau de la fonction, par défaut le htmlspecialchars enregistre pas en UTF-8 ?

Merci pour la suppression du stripslashes :) !

Pour le mot de passe, en gros je dois créer une variable qui change automatiquement en fonction des utilisateurs, que j'insert dans la BDD et que j'utilise via un "password_hash($password,$salt)" et ça sera protégé le plus correctement contre la bruteforce et autre ?

Je sais qu'en mysql fallait fermer les requêtes, j'utilisais donc closeCursor mais alors en PDO faut les laisser ouverte ?

J'ai pas compris pour la vérification du pseudo a partir de : "Faire l'INSERT directement et de vérifier si il s'est fait correctement en vérifiant les eventuels codes d'erreur renvoyé par la bdd.", désolé.

Du coup pour les erreurs, le mieux c'est l'idée de nestecha pour la fonction ou celle du tableau ?

Merci pour tous ces commentaires, cordialement,

Quand j’emploie le terme de « vue » je généralise mais je ne pensais pas précisément au pattern MVC. La vue c’est effectivement la partie affichage. Tu peux l’appeler vue, template, layout ou simplement affichage mais dans tous les cas on parle bien de la même chose : la génération de ton HTML.

D'accord

Tu mélanges un peu tout… pour résumer simplement : quand tu affiches, dans ta page HTML des données provenant d’une source externe, c'est-à-dire : un utilisateur, une API externe, ou n’importe quelle autre source, tu dois échapper les caractères spéciaux HTML. Ce n’est pas utile, c’est OBLIGATOIRE.
Après, pour le côté pratique je t’ai bien dit de partir sur une fonction helper ou sur un moteur de template pour te simplifier la vie.

Oui, mais justement si j'enregistre directement dans ma BDD les informations, je n'ai pas besoin de devoir les ressortir avec une fonction helper vu qu'ils sont enregistrés de cette façon; donc justement c'est là où je me pose la question : "est-ce utile de devoir le faire à chaque fois qu'on met quelque chose sortant de la BDD", alors qu'à l'enregistrement on peut très bien le faire une seule fois ?

Ça dépend de la version de PHP que tu utilises.

La version est 5.4.4-14+deb7u12

Oui, tu génères un sel unique par utilisateur et tu le stockes dans ta BDD. Non, ça ne protège pas contre les attaques par brute force mais contre les attaques par rainbow tables. Je te laisse relire mes explications et faire quelques recherches sur le sujet.

D'accord, par contre mes mots de passe actuels seront invalide du fait que même si j'insert dans toutes les lignes actuelle mon salt actuel, vu que la procédure n'est probablement pas la même ça ne fonctionnera pas ? :/

Tu parles de fermer la connexion ? Avec l’extension mysql ça n’a jamais été obligatoire et avec PDO non plus. La connexion à la base de données est fermée automatiquement en fin de script. De toute manière closeCursor ne sert pas du tout à fermer la connexion.
Si tu veux explicitement fermer la connexion avec PDO tu fais simplement : unset($mon_objet_pdo). Mais c’est pour des cas particuliers puisque que comme je te l’ai dit la connexion est fermée automatiquement à la fin de l’exécution de ton script PHP.

Excellent à savoir, merci.

Je reviendrais sur ce point une autre fois, je suis un peu pressé par le temps à l’instant.

J'attends ça avec impatience du coup :p !

Les deux en fait. Une fonction ou une classe te permettra d’extraire ton code et de le rendre réutilisable et le tableau n’est qu’une structure de données (la plus courante en PHP) qui sera alimentée par ta fonction. Les deux ne sont pas contradictoires.

D'accord, crois-tu qu'il est possible d'utiliser les deux méthodes sans perde de rapidité ou autre ? Si non, laquelle est la plus "optimisé" selon-toi ?

Merci pour tout :) !

J'en profite pour demander également au niveau du modèle MVC, lorsque nous avons des codes comme (exemple qui sert totalement à rien comme ça, mais le principe est bien représenté) :

<?php
if (isset($_GET'u']) && (strlen($_GET'u']) <= 34)){
    $pseudo = htmlspecialchars($_GET'u']);
    // Code du ?u=$pseudo
    echo '<span style="color:red">' , $pseudo , '</span>';
}else{
    // Pseudo non-renseigné  
    echo '<span style="color:red">Aucun pseudo renseigné.</span>';
}
?>

Comment faire pour séparer les codes ? Dans ma vue je devrai faire un code comme celui-ci ?

<?php if (!empty($pseudo)) { ?>
    <span style="color:red"><?php echo $pseudo; ?></span>
<?php }else{ ?>
    <span style="color:red">Aucun pseudo renseigné.</span>
<?php } ?>

Vous conseillez de faire des " if(){ ... }else{ ... } " ou bien " if: ... else: ... endif; " d'ailleurs ? Ah aussi, lorsqu'on crée des requêtes préparés, le mieux ce serait

$requete->bindValue(':nom_utilisateur', $nom_utilisateur);

ou

$requete->execute(array('nom_utilisateur' => $nom_utilisateur));

Désolé pour toutes ces questions :p

Un petit up :p

La structure MVC est pas si compliqué à comprendre en faite:
M odèle : Tout se qui est en relation avec la base de donnée; l'enregistrement/la sélection/la mise à jour des données.
V ue : Toute la partie où tu affiche tes données avec l'HTML. (Ces données sont envoyées par le controller en question.)
C ontrôleur : Toute la logique de l'application; réception de tes variables $_GET/$_POST, vérification des droits de l'utilisateur, etc etc comme par exemple ce bout de code :

if ($pseudo == NULL || $password == NULL || $repeat_password == NULL || $prenom == NULL || $email == NULL || $age == NULL){
    ++$erreur;
}

Concernant le routing du MVC, c'est simple aussi :

/controller/action/:parametre1/:parametre2 etc etc

Tu trouveras un beau tuto de Grafikart sur le MVC ici.

Vous conseillez de faire des " if(){ ... }else{ ... } " ou bien " if: ... else: ... endif; " d'ailleurs ?

La première est plus approprié pour être utilisé dans un fichier où il y a que du code php, comme dans un contrôleur par exemple.
Quand à la 2ième elle est parfaite pour être utilisé dans les vues. Car visuellement parlant, on la repère mieux dans du code HTML du faite des balises de fermetures : (Bien mieux que une simple accolade)

<?php endif; ?>
<?php endforeach; ?>
<php endwhile; ?>
etc

Ah aussi, lorsqu'on crée des requêtes préparés, le mieux ce serait

Il y a pas de mieux ou moins bien, il faut utiliser la méthode approprié à nos besoin.

Tu trouveras toutes les infos sur la doc php :
bindParam()
bindValue()
execute()
Constantes PDO

Cordialement.

Excellent, si j'ai une fonction pour les formulaires du type (voir code) je la met dans le contrôleur n'ayant aucun rapport avec la BDD du coup ? (PS: Si possible de commenter ce code également <3 ? Au passage, actuellement je cherche la raison du pourquoi lorsque je veux afficher mon </from> de ma dernière fonction, ça ne fonctionne pas xD):

// --------------------------------------------------
// -------------------- GESTION DES FORMULAIRES (PARTIE HTML)
// --------------------------------------------------
class htmlFormulaire{

    // Fonction pour afficher le <from> du début et attribut à ajouter
    public function ouvrir($name, $method = 'post', $action = '', $autre = ''){
        // Ajout de l'action si celle-ci est renseignée
        if ($action === ''){
            $ligne = '<form action=""' ;
        }else if ($action !== ''){
            $ligne = '<form action="' . $action . '" ';
        }else{
            $ligne = '<form action="" ';
        }
        // Code de départ obligatoire
        $ligne .= 'name="' . $name . '" ';
        // Ajout de la methode si elle est diffèrente de celle par defaut
        if ($method === ''){
            $ligne .= 'method="post" ';
        }else if ($method !== 'post'){
            $ligne .= 'method="' . $method . '" ';
        }else{
            $ligne .= 'method="post" ';
        }
        // Ajout des autres possibilités si elles sont déclarées
        if ($autre !== ''){
            $ligne .= $autre;
        }
        # if ($token !== ''){
        # $ligne .= '><input type="hidden" name="token" value="' . $_SESSION'token'] . '" />';
        # }else{
        # $ligne .= '>';
        # }
        echo $ligne . '>';
    }

    // Fonction pour afficher les champs inputs du formulaire choisi
    public function input($titre, $name = '', $type = 'text', $maxlength = '255', $autre = ''){
        // Name automatique et remplace les espaces, les accents et certains mots si le name est vide
        if ($name === ''){
            $name = strtolower($titre);
            $name = htmlentities($name, ENT_NOQUOTES, 'UTF-8');
            $name = preg_replace('#&([A-za-z])(?:acute|cedil|caron|circ|grave|orn|ring|slash|th|tilde|uml);#', '\1', $name);
            $name = preg_replace('#&([A-za-z]{2})(?:lig);#', '\1', $name);
            $name = preg_replace('#&^;]+;#', '', $name);
            $name = str_replace('le ', '', $name);
            $name = str_replace('votre ', '', $name);
            $name = str_replace('vos ', '', $name);
            $name = str_replace(' ', '_', $name);
        }
        // Code de départ obligatoire
        $ligne = '<strong>' . $titre . '</strong>
                 <input type="' . $type . '" name="' . $name . '" maxlength="' . $maxlength . '" ';
        // Ajout des autres possibilités si elles sont déclarées
        if ($autre !== ''){
            $ligne .= $autre;
        }
        echo $ligne . '><br>';
    }

    // Fonction pour afficher les textarea du formulaire choisi
    public function textarea($titre, $name = '', $value='', $placeholder = '', $autre = ''){
        // Name automatique et remplace les espaces, les accents et certains mots si le name est vide
        if ($name === ''){
            $name = strtolower($titre);
            $name = htmlentities($name, ENT_NOQUOTES, 'UTF-8');
            $name = preg_replace('#&([A-za-z])(?:acute|cedil|caron|circ|grave|orn|ring|slash|th|tilde|uml);#', '\1', $name);
            $name = preg_replace('#&([A-za-z]{2})(?:lig);#', '\1', $name);
            $name = preg_replace('#&^;]+;#', '', $name);
            $name = str_replace('le ', '', $name);
            $name = str_replace('votre ', '', $name);
            $name = str_replace('vos ', '', $name);
            $name = str_replace(' ', '_', $name);
        }
        // Code de départ obligatoire
        $ligne = '<strong>' . $titre . '</strong>
                 <textarea name="' . $name . '" ';
        // Ajout d'un placeholder s'il est déclaré
        if ($placeholder !== ''){
            $ligne .= 'placeholder="' . $placeholder . '" ';
        }
        // Ajout des autres possibilités si elles sont déclarées
        if ($autre !== ''){
            $ligne .= $autre;
        }
        $ligne .= '>';
        // Ajout d'un message directement dans le texteara s'il est déclaré
        if ($value !== ''){
            $ligne .= $value;
        }
        echo $ligne . '</textarea><br>';
    }

    // Fonction pour afficher le bouton submit du formulaire choisi
    public function submit($name = 'envoyer', $value = 'Envoyer', $class = 'btn', $autre = ''){
        $ligne = '<input type="submit" name="' . $name . '" value="' . $value . '" calss="' . $class . '"';
        // Ajout des autres possibilités si elles sont déclarées
        if ($autre !== ''){
            $ligne .= $autre;
        }
        echo $ligne . '><br>';
    }

    // Fonction pour afficher le bouton reset du formulaire choisi
    public function reset($value = 'Mettre à zéro', $class = 'btn', $autre = ''){
        $ligne = '<input type="reset" value="' . $value . '" calss="' . $class . '"';
        // Ajout des autres possibilités si elles sont déclarées
        if ($autre !== ''){
            $ligne .= $autre;
        }
        echo $ligne . '>';
    }

    // Fonction pour afficher la fin du formulaire HTML
    public function fermer(){
        echo '</from>';
    }

}

. Bon, il me reste plus que la compréhension de ceci étant assez important:

Ta façon de récupérer le pseudo pour voir s'il est déjà présent en base avec l'insert est une mauvaise pratique car tu peux avoir des cas ou la verification foire. La bonne méthode est d'avoir un INDEX de type UNIQUE sur ta colonne pseudo, de faire l'INSERT directement et de vérifier si il s'est fait correctement en vérifiant les eventuels codes d'erreur renvoyé par la bdd.

Encore merci !

Bonsoir,

C'est form et non from pour ta dernière fonction ;)

Cordialement

< Quelle faute ridicule ! Donc me reste plus que savoir si ça doit bien aller dans contrôleurs, si vous avez des conseils a propos de ces fonctions et peut-être plus d'informations sur "Ta façon de récupérer le pseudo pour voir s'il est déjà présent en base avec l'insert est une mauvaise pratique car tu peux avoir des cas ou la verification foire. La bonne méthode est d'avoir un INDEX de type UNIQUE sur ta colonne pseudo, de faire l'INSERT directement et de vérifier si il s'est fait correctement en vérifiant les eventuels codes d'erreur renvoyé par la bdd."
Merci =)

Petite remonte :p

Encore une petite remonte :p

De même :p

Salut!!

d'abord sur le hash des mots de passe, comme l'a dit rapidement sudovim, sha1 ne doit plus être utilisé, il y a des fonctions de hash plus récentes comme bcrypt. Le salt doit être long , et le mieux est d'avoir un salt différent pour chaque utilisateur (mais bon ça déjà c'est moins grave)
Salted Password Hashing - Doing it Right

Ensuite sur l'utilisation d'outils, même si on "utilise 3 fichiers pour une page" il faut savoir qu'en programmation ça vaut toujours le coup de perdre quelque pourcent de performance au profit d'un code plus propre, plus lisible, plus facile à maintenir et souvent plus modulaire.

Je te conseille de lire souvent les actus par exemple sur reddit.com/r/programming (ou /r/php ou d'autre reddit qui t'intéresse) tu y trouvera plein de conseil interessant donnés par des pros (il y a pas mal d'article sur le coding style dans les archives qui pourront t’intéresser, et comme cela intéresse beaucoup de monde, ils sont souvent bien notés) .

D'accord, merci.
Savez-vous comment " vérifier les eventuels codes d'erreur renvoyé par la BDD " ?

up

Bonjour en PDO $e->getMessage(); depuis le try{}catch(PDOException $e){}
Cordialement.