Voici donc mes remarques:
- On passera donc les remarques sur l'organisation qui est une catastrophe
htmlspecialchars, ça s'utilise uniquement à l'affichage, comme expliqué ici
Or toi, tu fais tes html specialchars....... partout. Même pour simplement regarder dans la bdd (faire un select) ou pour… jouer avec des entiers.
D'ailleurs revenons à ce fichier : tu supposes que $_GET['id'] est un entier qui vaut entre 1 et 4 inclus. Mais si je mets "Ikillyou" ou même 42, tout ce que tu auras c'est "Warning $width is not defined" (idem pour height).
$d = date('Y-m-d'); est une mauvaise pratique car tu ne sais pas quelle est la timezone que tu utilises.
| <?php
$today = new DateTime("today", new DateTimezone("Europe/Paris"));
$date_string = $today->format("Y-m-d");
|
te permet de t'assurer de tout ça tranquillement.
Toujours dans les mauvaises pratiques, tes nommages de variables sont.... WTF?
| <?php
$one = $pdo->prepare('SELECT id_prop FROM site WHERE ID=:id');
$one->bindValue(':id', $id_aff, PDO::PARAM_INT);
$one->execute();
$ones = $one->fetch();
$id_prop = $ones['id_prop'];
|
Qui peut comprendre ce que tu fais avec ton $one et $ones qui ne présentent rien du tout. De plus, comme tu n'utilises pas la propriété PDO::FETCH_ASSOC, il me semble que tu charges les choses en double (équivalent à PDO::FETCH_BOTH). Ce qui n'est pas une bonne méthode. (par contre pas sûr quoi).
Je ne peux pas commenter les lignes qui suivent car sans commentaire ni nom de variable utile, ton code est juste… illisible.
| <?php
for ($nb_lignes = 0; $nb_lignes <= $donnees['score']; $nb_lignes++)
{
$liste[] = $donnees['ID'];
$nb_lignes ++;
}
|
what the hell? si tu veux vraiment incrémenter ton compteur par deux, pourquoi tu ne fais pas un $nb_lignes+=2 dans ton for? Et sinon, pourquoi avoir fait ça? Surtout que j'ai l'impression que tu veux juste faire un array_fill, qui est une fonction déjà existante.
Tu remarqueras que je n'ai toujours pas quitté le ficher "add.php" qui semble être un fichier certes important mais très buggé. Où est le découpage en fonctions de ton code?
| <?php
$id = $liste[rand(0, count($liste)-1)];
if (!empty($id)){
|
quel sens donnes-tu à ça? Cette condition ne peut échouer que si $id vaut 0. Or si $id est bien tiré d'une bdd c'est… impossible.
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23 | <?php
$reponse = $pdo->prepare('SELECT * FROM vues WHERE id_site=:id AND dat=:dat');
$reponse->bindValue(':id', $id, PDO::PARAM_INT);
$reponse->bindValue(':dat', $d, PDO::PARAM_STR);
$reponse->execute();
$donnees = $reponse->fetch();
$reponse->closeCursor();
if (!empty($donnees)){
$vues = $donnees['nb'];
$vues ++;
$reponse = $pdo->prepare('UPDATE vues SET nb=:vues WHERE id_site=:id AND dat=:dat');
$reponse->bindValue(':vues', $vues, PDO::PARAM_INT);
$reponse->bindValue(':id', $id, PDO::PARAM_INT);
$reponse->bindValue(':dat', $d, PDO::PARAM_STR);
$reponse->execute();
$reponse->closeCursor();
}else{
$reponse = $pdo->prepare('INSERT INTO vues(nb, id_site, dat) VALUES(1, :id, :dat)');
$reponse->bindValue(':id', $id, PDO::PARAM_INT);
$reponse->bindValue(':dat', $d, PDO::PARAM_STR);
$reponse->execute();
$reponse->closeCursor();
}
|
la requête de sélection est en trop.
Il te suffit d'envoyer la requête UPDATE vues SET nb=nb+1 WHERE id_site=:id AND dat=:dat et de regarder le retour de la fonction execute. Si ce dernier est égal à 0 alors tu fais on INSERT, sinon tu peux rester.
Autre aspect : tu n'utilises pas les transactions, du coup tu es sensible aux accès concurrent. Et tu auras un jour ou l'autre des warning pour variable inexistante.
L'utilisation de die pour un oui ou pour un non est aussi une mauvaise pratique, tu ne veux pas que la génération de ta page meurt, tu veux qu'en cas d'erreur elle s'adapte.
echo '<META HTTP-EQUIV="Refresh" CONTENT="0;URL= /dashboard">';
NON !
| header('Location: /dashboard');
|
et si possible sans avoir les include bas et haut qui ne servent à rien.
Comme tes fichiers ne sont pas organisés, on peut accéder à des fichiers intermédiaires sans aucun soucis. Passons à inscript.php
| <?php
if ((gettype($pseudo) == "string") AND (gettype($mail) == "string") AND (gettype($mdp) == "string") AND (gettype($verif) == "string"))
|
Pourquoi? OSEF, non? outre que la fonction is_str existe ça n'a aucun intérêt puisque :
- tu te sers directement des $_POST qui sont TOUJOURS des strings (sauf quand ils n'existent pas… ce que tu ne vérifie pas)
- tu utilises (très mal mais tu l'utilise quand même) htmlspecialchars qui retourne un string.
md5 n'est pas sécurisé pour stocker des mots de passes.
J'ajouterai que l'unicité d'une clef (utilisateur, email) c'est le travail de la bdd, pas du php. Il faut donc que tu ajoutes une contrainte d'unicité et que tu insères directement (sans vérification). S'il y a doublon, la bdd renvoie une erreur que tu peux attraper (try/catch).
En plus cela accélèrera ta bdd.
Je n'ai pas tout inspecté tes fichiers car il y en a trop, trop mal rangés, avec un code trop mal rédigé. J'espère que mes commentaires te serviront à t'améliorer.
En tout cas, tu as bien compris comment utiliser les requêtes préparées et tu n'es pas vulnérable aux injections SQL. C'est plutôt une bonne chose.
)
