Mon code est il bon ?

Version imprimable

Bonjour alors j'aimerais vous exposer mon code ici pour voir ce que vous en pensiez. Failles, propretés etc...
Avant tout, j'aimerais que vous m'éclaircissiez sur un point.
Le :

Code:

mysql_query("SET NAMES 'utf8'");

Est il obligatoire pour prévenir le serveur MYSQL que la transaction des données ce fait en UTF8 ?

Pas page enregistrement (pour ce qui est des protections htmlspecialschars, je le fais à l'affichage) :

Code:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
<?php
include ("connect.php");
$sql = "SELECT * FROM quetes WHERE titre='".$_POST['titre']."'";
$result = mysql_query($sql);
if(mysql_num_rows($result) > 0) // Vérification si il existe déjà.
{
echo 'La quêtes existe déjà';
}
else // Sinon, on enregistre
{
if (!empty ($_POST['titre']) && is_numeric($_POST['type']) && is_numeric($_POST['faction']) && is_numeric($_POST['classe']) && !empty ($_POST['de']) && is_numeric($_POST['lvl']) && !empty ($_POST['objectif']) && !empty ($_POST['aide']) && is_numeric($_POST['xp']) && is_numeric($_POST['kinah']))
{
include ("connect.php");
$sql = "INSERT INTO quetes VALUES ('', '".mysql_real_escape_string($_POST['titre'])."', '".($_POST['type'])."', '".($_POST['faction'])."', '".($_POST['classe'])."', '".mysql_real_escape_string($_POST['de'])."', '".($_POST['lvl'])."', '".mysql_real_escape_string($_POST['objectif'])."', '".mysql_real_escape_string($_POST['aide'])."', '".($_POST['xp'])."', '".($_POST['kinah'])."')";
mysql_query ($sql);
echo $sql;
}
else
{
echo 'Vous n\'avez pas remplis tout les champs, ou vous avez fait une erreur...';
}
}
?>

Maintenant l'affichage :

Code:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
<?php
// Affichage des quêtes de la faction ELYSEENS !
if (isset ($_GET['Faction']) AND isset ($_GET['Classe']))
{
	if (($_GET['Faction'] == 'Elyseens') OR ($_GET['Faction'] == 'Asmodiens') OR ($_GET['Faction'] == 'E/A') OR ($_GET['Classe'] == 'Tout') OR ($_GET['Classe'] == 'Mage') OR ($_GET['Classe'] == 'Prêtre') OR ($_GET['Classe'] == 'Guerrier') OR ($_GET['Classe'] == 'Éclaireur'))
	{
			if ($_GET['Faction'] == 'E/A')
			{
			$faction = 0;
			}
			if ($_GET['Faction'] == 'Elyseens')
			{
			$faction = 1;
			}
			if ($_GET['Faction'] == 'Asmodiens')
			{
			$faction = 2;
			}
			if ($_GET['Classe'] == 'Tout')
			{
			$classe = 0;
			}
			if ($_GET['Classe'] == 'Mage')
			{
			$classe = 1;
			}
			if ($_GET['Classe'] == 'Prêtre')
			{
			$classe = 2;
			}
			if ($_GET['Classe'] == 'Guerrier')
			{
			$classe = 3;
			}
			if ($_GET['Classe'] == 'Éclaireur')
			{
			$classe = 4;
			}
		include ("connect.php");
		$sql = "SELECT * FROM quetes WHERE faction=".$faction." AND classe=".$classe;
		$result = mysql_query($sql);
		while ($rep = mysql_fetch_array($result))
		{
		echo 'Titre : '.$rep['titre'].'<br />';
		}
	}
}
// Affichage => FIN
 
else
{
?>
<a href="quetes.php?Faction=Elyseens&Classe=Tout">Toutes les quêtes Elyseens</a><br />
<a href="quetes.php?Faction=Elyseens&Classe=Mage">Elyseens/Mage</a><br />
<a href="quetes.php?Faction=Elyseens&Classe=Prêtre">Elyseens/Prêtre</a><br />
<a href="quetes.php?Faction=Elyseens&Classe=Guerrier">Elyseens/Guerrier</a><br />
<a href="quetes.php?Faction=Elyseens&Classe=Éclaireur">Elyseens/Éclaireur</a><br /><br />
<a href="quetes.php?Faction=Asmodiens&Classe=Tout">Toutes les quêtes Asmodiens</a><br />
<a href="quetes.php?Faction=Asmodiens&Classe=Mage">Asmodiens/Mage</a><br />
<a href="quetes.php?Faction=Asmodiens&Classe=Prêtre">Asmodiens/Prêtre</a><br />
<a href="quetes.php?Faction=Asmodiens&Classe=Guerrier">Asmodiens/Guerrier</a><br />
<a href="quetes.php?Faction=Asmodiens&Classe=Éclaireur">Asmodiens/Éclaireur</a><br />
<?php
}
?>

Qu'en pensez vous ? Merci

Tu pourrais un peu alléger le deuxieme script :

Code:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
<?php
if (isset ($_GET['Faction']) AND isset ($_GET['Classe'])) {
	switch ($_GET['Faction']) {
		case 'E/A' : $faction = 0; break;
		case 'Elyseens' : $faction = 1; break;
		...
	}
 
	switch ($_GET['Classe']) {
		case 'Tout' : $classe = 0; break;
		case 'Mage' : $classe = 1; break;
		....
	}
 
	if (isset($classe) && isset($faction)) {
		include ("connect.php");
		$sql = "SELECT * FROM quetes WHERE faction=".$faction." AND classe=".$classe;
		$result = mysql_query($sql);
		while ($rep = mysql_fetch_array($result)) {
			echo 'Titre : '.$rep['titre'].'<br />';
		}
	}
}

Ou utiliser des elseif{}

Code:

$sql = "INSERT INTO quetes VALUES ('', '"

Il vaut mieux indiquer le nom des champs et ne pas mettre une chaine vide pour l'autoincrementation, certains configurations de mysql ne l'accepte pas.

Je mets en commentaire les critiques que je peux apporter au script...

Code:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
<?php
include ("connect.php"); // Pas besoin de " autour d'un littéral, des ' suffisent
 
// Pas besoin de " autour d'un littéral, utilise " ou immerge $_POST dans la chaîne
// $_POST n'est pas échappée, magic-quotes ?
$sql = "SELECT * FROM quetes WHERE titre='".$_POST['titre']."'";
 
$result = mysql_query($sql);
 
// Si la requête sert à tester l'existence d'un enregistrement
// Alors inutile de faire un SELECT *, un SELECT COUNT( ) suffit
if(mysql_num_rows($result) > 0)
{
echo 'La quêtes existe déjà';
}
else
{
// J'aurais d'abord tester la validité des champs avant de faire appel au serveur MySQL
// empty( ) n'est pas adapté pour tester si un champ a été rempli ou non
// is_numeric( ) n'est pas adapté pour tester si une valeur est un entier ou non
if (!empty ($_POST['titre']) && is_numeric($_POST['type']) && is_numeric($_POST['faction']) && is_numeric($_POST['classe']) && !empty ($_POST['de']) && is_numeric($_POST['lvl']) && !empty ($_POST['objectif']) && !empty ($_POST['aide']) && is_numeric($_POST['xp']) && is_numeric($_POST['kinah']))
{
// Tu rappelles une 2nde fois connect.php
include ("connect.php");
// Tu n'échappes pas toutes les valeurs envoyées à MySQL
$sql = "INSERT INTO quetes VALUES ('', '".mysql_real_escape_string($_POST['titre'])."', '".($_POST['type'])."', '".($_POST['faction'])."', '".($_POST['classe'])."', '".mysql_real_escape_string($_POST['de'])."', '".($_POST['lvl'])."', '".mysql_real_escape_string($_POST['objectif'])."', '".mysql_real_escape_string($_POST['aide'])."', '".($_POST['xp'])."', '".($_POST['kinah'])."')";
mysql_query ($sql);
// Bien pour le débugage uniquement
echo $sql;
}
else
{
echo 'Vous n\'avez pas remplis tout les champs, ou vous avez fait une erreur...';
}
}
?>

+ Code non indenté = difficilement lisible :(

Pour le 2nd script tu peux remplacer la 1re partie par :

Code:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
 
if ( isset($_GET['Faction'], $_GET['Classe']) )
{
        $factions = array(1 => 'Elyseens', 2 => 'Asmodiens', 0 => 'E/A') ;
        $classes = array(0 => 'Tout', 1 => 'Mage', 2 => 'Prêtre', 3 => 'Guerrier', 4 => 'Éclaireur') ;
 
        $faction = array_search($_GET['Faction'], $factions) ;
        $classe = array_search($_GET['Faction'], $classes) ;
 
        if ( $faction !== FALSE && $classe !== FALSE ) {
 
            include ("connect.php");
 
            // Eviter les SELECT * si on n'utilise pas tous les champs
            $sql = "SELECT titre FROM quetes WHERE faction=".$faction." AND classe=".$classe;
            $result = mysql_query($sql);
 
            while ($rep = mysql_fetch_array($result)) {
                echo 'Titre : '.$rep['titre'].'<br />';
            }
        }
}
else ...

13/05/2009, 17h11
sabotage

J'ai reflechi (et oui) à ce is_numeric() :

si l'utilisateur passe autre chose que des chiffres, ca ne passera pas ; si l'utilisateur passe un decimal ou un chiffre avec des zeros au début, ca passera mais mysql ne gardera que la valeur entiere (sous reserve d'utiliser les guillemets).

Donc finalement n'est ce pas suffisant comme test ? Et du coup inutile d'echapper la chaine.
13/05/2009, 17h23
Sheiya

Citation:

Envoyé par sabotage

Donc finalement n'est ce pas suffisant comme test ? Et du coup inutile d'echapper la chaine.

J'ai pas compris ce que tu essaye de me dire la :p

Ensuite

Citation:

// empty( ) n'est pas adapté pour tester si un champ a été rempli ou non

Alors je met quoi ? ^^

Citation:

// Tu n'échappes pas toutes les valeurs envoyées à MySQL

Ce qui veut dire ? (si c'est is_numeric, unitule de faire le test non , )

Sinon,

Code:

mysql_query("SET NAMES 'utf8'");

Est il obligatoire pour prévenir le serveur MYSQL que la transaction des données ce fait en UTF8 ?
13/05/2009, 17h34
sabotage

Citation:

Est il obligatoire pour prévenir le serveur MYSQL que la transaction des données ce fait en UTF8 ?

Est-ce que cela te coute de le mettre ?
Cela t'assure que la connexion se fait dans l'encodage que tu souhaites independemment de la configuration du serveur.
13/05/2009, 17h45
Séb.

Citation:

Citation:

// empty( ) n'est pas adapté pour tester si un champ a été rempli ou non

Alors je met quoi ? ^^

empty('0') => TRUE alors que la chaîne n'est pas vide http://fr.php.net/empty

Amha mieux vaux faire :

Code:

if ( $tonChamp === '' ) { // Si rien n'a été saisi

Code:

if ( $tonChamp !== '' ) { // Si qquechose a été saisi

Ensuite :

Citation:

Citation:

Citation:
// Tu n'échappes pas toutes les valeurs envoyées à MySQL

Ce qui veut dire ? (si c'est is_numeric, unitule de faire le test non , )

Certes, mais il faudrait que tu aies 100% confiance en is_numeric( ) (en restant raisonnable).
La sécurité est affaire de non-confiance :mouarf:

Citation:

si l'utilisateur passe autre chose que des chiffres, ca ne passera pas ;

Si si ça passera car is_numeric( ) accepte les notations scientifique et hexadécimale, dixit la doc.
13/05/2009, 19h31
Sheiya

Citation:

Si si ça passera car is_numeric( ) accepte les notations scientifique et hexadécimale, dixit la doc.

Ma table est en Tynint je risque logiquement rien.
13/05/2009, 19h52
Séb.

Tout dépend de ta définition du risque.