Bonjour,

Voila je rencontre un petit problème avec mon code.

J'ai suivi le cours sur la POO en PHP, je réécris les classes Table et Database.
Voici la classe Table :

<?php 
class Table
{
    private $db;

    public function __construct(Database $db)
    {
        $this->db = $db;
    }

    public function query($statement, $attributes = null, $one = true)
    {
        if(is_null($attributes))
        {
            $req = $this->db->query($statement, $one);
        }
        else
        {
            $req = $this->db->prepare($statement, $attributes, $one);
        }
        return $req;
    }

    public function getByDesc($table, $by, $one = true)
    {
        $req = $this->query("SELECT * FROM ? ORDER BY ?DESC", [$table, $by], $one);
        return $req;
    }

    public function getByAsc($table, $by, $one = true)
    {
        $req = $this->query("SELECT * FROM ? ORDER BY ? ASC", [$table, $by], $one);
        return $req;
    }
}

 ?>

Et la classe Database :

<?php 
class Database
{
    private $db_name;
    private $db_host;
    private $db_user;
    private $db_pass;
    private $pdo;
    /**
     * Set all the variables for the connection to the database
     * @param string $db_name Name of the database
     * @param string $db_user Username for connection to the database
     * @param string $db_pass Password for connection to the database
     * @param string $db_host Host of the database
     */
    public function __construct($db_name,  $db_host = 'localhost', $db_user = 'root', $db_pass = '')
    {
        $this->db_name = $db_name;
        $this->db_user = $db_user;
        $this->db_pass = $db_pass;
        $this->db_host = $db_host;
    }
    /**
     * Create a new intance of PDO or use the actual instance
     * @return Object A instance of PDO
     */
    private function getPDO()
    {
        if(is_null($this->pdo))
        {
            try
            {
            $this->pdo = new PDO('mysql:host=' . $this->db_host . ';dbname=' . $this->db_name, $this->db_user, $this->db_pass);
            $this->pdo->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
            }
            catch(PDOException $e)
            {
                echo 'Error : ' . $e->getMessage();
            }
        }
        return $this->pdo;
    }

    /**
     * Send a request to get, update or delete dateas without attributes
     * @param  string  $statement The request
     * @param  boolean $one       Select if we only will show one result
     * @return mixed   $result    The results of the request
     */
    public function query($statement, $one = true)
    {
        $req = $this->getPDO()->query($statement);
        if($one == true)
        {
            $result = $req->fetch();
        }
        else
        {
            $result = $req->fetchAll();
        }
        return $result;
    }

    /**
     * Send a request to get, update or delete dateas with attributes
     * @param  string  $statement The request
     * @param  array   $attributes The array for the execute
     * @param  boolean $one       Select if we only will show one result
     * @return mixed   $result    The results of the request
     */
    public function prepare($statement, $attributes = [], $one = true)
    {
        $req = $this->getPDO()->prepare($statement);
        try {   
        $req->execute($attributes);
        } catch (PDOException $e) {

            echo $e;
        }
        if($one == true)
        {
            $result = $req->fetch();
        }
        else
        {
            $result = $req->fetchAll();
        }
        return $result;

    }
}

Je chercherais, avec les fonctions getByDesc et getByAsc, pouvoir récupérer les éléments d'une table avec un ordre précis.
Donc, je passe $table qui est le nom de la table où l'on récupère les éléments et $by selon quoi on classe les éléments (id, date, etc).
Je me demande donc si ma façon de passer est bonne (bien que je reçois une erreur, j'en parle plus bas) ou si je peux directement passer ça dans le string qui est ma requête ($statement).
Le problème avec la deuxième manière de procéder, c'est que je ne suis pas sûr s'il y a un risque d'injection SQL même si l'utilisateur ne pourra pas entrer les informations nécéssaires à la fonction.

Pour la première manière de procéder, je reçois l'erreur :
exception 'PDOException' with message 'SQLSTATE[42000]: Syntax error or access violation: 1064 Erreur de syntaxe pr�s de ''actions' ORDER BY 'id' ASC' � la ligne 1' in C:\wamp64\www\mods\Table\Class\Database.php:75 Stack trace: #0 C:\wamp64\www\mods\Table\Class\Database.php(75): PDOStatement->execute(Array) #1 C:\wamp64\www\mods\Table\Class\Table.php(19): Database->prepare('SELECT FROM ?...', Array, true) #2 C:\wamp64\www\mods\Table\Class\Table.php(32): Table->query('SELECT FROM ?...', Array, true) #3 C:\wamp64\www\mods\Table\index.php(50): Table->getByAsc('actions', 'id') #4 {main}

Pour la seconde, dont je ne suis pas sûr à cause de la sécurité, tout marche sans problème.

Du coup, pourriez-vous me conseiller sur ce qui est le mieux niveau sécurité et m'aider à résoudre mes erreurs ?

Merci d'avance à tous et à toutes !
Si vous avez besoin de plus de précisions, n'hésitez pas ! :)

2 réponses


Metrogeek
Réponse acceptée

Hello @TheFlameflo,

Je ne suis pas certain de comprendre tes 2 manières de procéder. Si tu parles de 2 méthodes des classes qui sont en exemple, est-ce que tu peux les nommer directement ?

Si je comprends bien, et si mes souvenirs sont bons, tu fais un mauvais usage du 2e paramètre dans la fonction preprare() de PDO. Tu es sensé envoyer des options liées au driver et non pas le reste de ta requete.

Aussi, tu as des points d'interrogation dans ta requête, au lieu d'utiliser les paramètres de ta méthode :

 $req = $this->query("SELECT * FROM ? ORDER BY ? ASC", [$table, $by], $one);

A mon avis, tu voulais ça :

  $req = $this->query("SELECT * FROM {$table} ORDER BY {$by} ASC");

Tu fais beaucoup de try/catch avec du echo. Je déconseille vraiment, dans la mesure où sans utilisation du return, ton application va aller plus loin, en retournant un nouveau message d'erreur (propriété non accessible ou autre) et tu risque d'afficher un message un peu confus au visiteur. Au lieu du echo, je ferais du return $e->getMessage(); ou encapsuler autrement pour l'utiliser dans la mise en forme d'un message d'erreur.

Pour la sécurité, globalement si tu utilises vraiment les requêtes préparées de PDO (ça va plus loin que la simple execution de la fonction, n'hésite pas à lire la doc pour aller plus loin) tu as quand même assez peu de risque pour ta base. Ca n'empêche pas de prendre des mesures, côté vue, et en validation de requête avant l'insertion/mise à jour dans la base.

Salut !

Ce que tu m'as proposé marche bien, merci !
Pour les try/catch, je les ai mis rapidement, sans prendre le temps de bien le faire, pour trouver les problèmes.
Je vais enlever ceux qui sont inutiles !

Merci beaucoup ! :)