La classe est aussi blindée de trous.
<?php
define ("CURRENT_PAGE", basename($_SERVER['PHP_SELF'])); // page courante
$_SERVER['PHP_SELF'] est une donnée provenant de l'extérieur. Il faut donc la contrôler plus que ça. Sinon, il peut y avoir desinjections d'HTML et de Javascript.
<?php
private function _getUrlOrdreListe($a_colone,$a_sens)
{
$lien = $_SERVER['QUERY_STRING'];
if (strlen($this->m_arguments)>0)
$lien .= ( strlen($lien)>0 ? "&" : "" ) . $this->m_arguments;
$lien = preg_replace("/&?ord=.*&sens=.*/","",$lien);
$lien .= (strlen($lien)>0 ? "&" : "") . "ord=".$a_colone."&sens=".$this->_setSens($a_sens,$a_colone);
return (CURRENT_PAGE."?".$lien);
}
Il manque les nombreux urlencode ou urldecode nécessaires dans la reconstruction de la query string.
L'expression régulière est boguée. Je sens les URL dans les quelles tu vas perdre des paramètres HTTP. En règle générale, il ne faut pas utiliser le point.
Une meilleure expression régulière serait :
'^?(?:ord=(?<ord>:[^&]*)|sens=(?<sens>:[^&]*)|(?:\w+=\w+))*$'
À partir de la fonction headList, tu fournis tout un tas de fonctions qui génèrent du HTML. Dans aucune d'entre elles tu ne protèges correctement les données que tu intègres au HTML avec
htmlentities
ou
htmlspecialchars.
D'ailleurs tu n'utilises jamais
preg_escape.
Les points suivants ne sont pas à proprement parler des problèmes de sécurité. Ils ont plus traits ux performances et aux bonnes pratiques.
Ton constructeur fait beaucoup d'initialisations alors qu'elles pourraient être faites dans la définition de la classe, au niveau de la définition des attributs.
Tu places les méthodes et variables public en fin de définition de la classe. Une bonne pratique consiste plutôt à placer tout ce qui est public en tête, pour faciliter l'adoption de la classe : l'utilisateur de la classe n'a pas besoin de connaître les détails intimes de la classe. Mais je te concède que c'est essentiellement esthétique.
Les méthodes telles que setPas() écrivent directement dans la session. Pourquoi ne pas le faire à la destruction de l'objet ?
Je remarque aussi pas mal de choses qui peuvent être écrites plus simplement, sans nuire à l lisibilité.
Par exemple, if($variable == true) se remplace par if($variable). if(isset($variable) && strlen($variable) > 0) se remplace par if(!empty($variable)). La fonction
empty peut s'appliquer sur les index de tableau indéfinis. Pareil pour if (count($matches[1])>0) qui peut être écrit if(!empty($matches[1])).
Appeler exit au milieu d'une classe, c'est mal. Surtout pour une condition invalide aussi ridicule qu'une page négative. Dans ce genre de cas, il vaut mieux forcer un nombre de page, ou lever une exception si aussi catastrophique. L'usage d'une exception permet à l'utilisateur de la classe d'intégrer plus facilement cette dernière. La pagination n'est pas forcément ce qu'il y a de plus important dans le déroulement de l'application développée avec ta classe.
Dans tout les cas, saupoudrer son code d'appels aux fonctions d'assassinat du processus n'est jamais une bonne idée, car cela rend le déboguage difficile et pénible.
Il y a aussi une mauvaise pratique au niveau de la génération de ton HTML, concernant le style. Tu imposes un style particulier au code produit, ce qui rend impossible l'intégration à une charte graphique, sans devoir modifier massivement le code de la classe. Il serait plus pertinent d'utiliser l'attribut class des éléments. Mais en fait, il y a la un gros problème d'architecture de ta classe.
Car malheureusement, comme beaucoup, tu crois qu'une classe est un module. C'est-à-dire que tu concentre toute l'intelligence de l'application dans une seule classe qui sait tout faire : gestion de l'URL, gestion de de la persistance des données, génération du code HTML, gestion du comportement, etc. Tout ces concepts devraient être encapsulés dans leurs propres classes, afin de permettre une meilleure modularisation, en permettant la personnalisation des composant. Par exemple, en découplant la création de la sortie HTML, ça permet à un utilisateur de ton système de dériver la classe et de personnaliser le comportement. Par exemple, il peut vouloir générer du XHTML2 ou de l'HTML5. Une autre classe serait encore la gestion de l'URL. Comment font ls utilisateurs qui jouent de l'URL Rewriting pour utiliser ta classe ?
Quand j'affirme qu'un code n'est pas sécurisé, j'ai raison dans 90% des cas.
Développeur récurrent, procédural et relationnel. Caustique soupe-au-lait.
