Votre avis sur la qualité d'un script

Version imprimable

Salut ;)

Je suis un débutant qui tente de s'améliorer et j'aurais aimer avoir votre avis sur un script tout bête d'insertion de données dans une table Mysql, donc si ça vous dit, tout vos commentaires seront les bienvenues !

C'est surtout au niveau des contrôles où j'aimerais votre opinion :

Voici le script sachant qu'il s'agit de variables récupérées via un formulaire :

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
<?php
 
/* Vérification des champs vides du formulaire de création d'une nouvelle intervention */
 
if(empty($_POST['dateinter_int']))
{
echo 'Le champs <b>"Date"</b> est vide.<br /> Cliquez <a href="javascript:history.back()">ici</a> pour revenir à la fiche d\'intervention';
exit();
}
if(empty($_POST['typeinter_int']))
{
echo 'Le champs <b>"Type d\'intervention"</b> est vide.<br /> Cliquez <a href="javascript:history.back()">ici</a> pour revenir à la fiche d\'intervention';
exit();
}
if(empty($_POST['tec_int']))
{
echo 'Le champs <b>"Technicien"</b> est vide.<br /> Cliquez <a href="javascript:history.back()">ici</a> pour revenir à la fiche d\'intervention';
exit();
}
if(empty($_POST['patientid_int']))
{
echo 'Le champs <b>"Patient"</b> est vide.<br /> Cliquez <a href="javascript:history.back()">ici</a> pour revenir à la fiche d\'intervention';
exit();
}
 
else{
 
 
	/* Création de la fonction de traitement de la date US vers FR  */
	function convert_date($date)
   {
      $tab_date = explode("-",$date);
      $retour = $tab_date[2]."-".$tab_date[1]."-".$tab_date[0];
 
      return $retour;
   }
 
	$dateinter = convert_date($_POST['dateinter_int']);
	$typinter = $_POST['typeinter_int'];
	$tec = $_POST['tec_int'];
	$patientid = $_POST['patientid_int'];
 
  $req = "INSERT INTO tinter (dateinter_int, typeinter_int, tec_int, patientid_int) VALUES('$dateinter', '$typeinter', '$tec', '$patientid')";
  mysql_query($req) or die(mysql_error()) ;
 
  echo "L'intervention à été enregistrée avec succès.";
 
  }
 
 
 
  ?>

Merci pour votre esprit critique ! ;)

Bonsoir...

Ton script me semble tout à fait correct. Cela dit, je me permet de te suggérer de remplacer tes if successifs par des "if ... else". Si par exemple l'utilisateur n'a pas saisi plusieurs champs, il ne verra apparaitre qu'une seule fois "Cliquez ici pour revenir à la fiche d'intervention"...

J'ai également re-écrit ton bout de code tel que je l'aurais fait si j'avais du faire la même chose... Si tu trouves qu'il y de bonnes idées tu peux te servir !

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
$saisieOK = true;
if(empty($_POST['dateinter_int']))
{
    $saisieOK = false;
    array_push($listWrong, "date");
}
if(empty($_POST['typeinter_int']))
{
    $saisieOK = false;
    array_push($listWrong, "type d\'intervention");
}
if(empty($_POST['tec_int']))
{
    $saisieOK = false;
    array_push($listWrong, "technicien");
}
if(empty($_POST['patientid_int']))
{
    $saisieOK = false;
    array_push($listWrong, "patient");
}
 
if($saisieOK)
{
    $dateinter = convert_date($_POST['dateinter_int']);
    $typinter = $_POST['typeinter_int'];
    $tec = $_POST['tec_int'];
    $patientid = $_POST['patientid_int'];
 
    $req = "INSERT INTO tinter (dateinter_int, typeinter_int, tec_int, patientid_int) VALUES('$dateinter', '$typeinter', '$tec', '$patientid')";
    if(mysql_query($req))
    {
        echo "L'intervention à été enregistrée avec succès.";
    }
    else
    {
        echo "Erreur lors de l'insertion de l'intervention.";
    }
}
else
{
    echo 'Le(s) champ(s) suivants sont vides :<br /> <ul>';
    for($i=0; $i < count($listWrong); $i++)
    {
         echo '<li>'.$listWrong[$i].'</li>';
    }
    echo '</ul>Cliquez <a href="javascript:history.back()">ici</a> pour revenir à la fiche d\'intervention';
 
}

En espérant t'avoir été utile... bonne continuation ! :P
Bonsoir...

30/06/2008, 09h30
rewsna
Bonjour,

je rajouterai un contrôle au niveau des variables pour éviter l'injection SQL et les failles XSS potentielles.
Le principe est le suivant : toujours contrôler les variables extérieurs au script/programme, n'accepter que ce qui est valide, rejeter le reste.
Code:

1 2 3 4 $typinter = htmlentities($_POST['typeinter_int'], ENT_QUOTES); $tec = htmlentities($_POST['tec_int'], ENT_QUOTES); $patientid = htmlentities($_POST['patientid_int'], ENT_QUOTES);
Il est possible de faire d'autres contrôles, comme le type de données attendu voir les méthodes is_*() ou ctype_*() pour cela.
Mais aussi la valeur attendue, un entier strictement positif par exemple, etc.
Code:

1 2 if (is_numeric($typinter) && $typinter > 0 ) { // traitement }
30/06/2008, 10h03
chtipitou
Citation:
Envoyé par rewsna

Bonjour,

je rajouterai un contrôle au niveau des variables pour éviter l'injection SQL et les failles XSS potentielles.
Le principe est le suivant : toujours contrôler les variables extérieurs au script/programme, n'accepter que ce qui est valide, rejeter le reste.

Code:

1 2 3 4 $typinter = htmlentities($_POST['typeinter_int'], ENT_QUOTES); $tec = htmlentities($_POST['tec_int'], ENT_QUOTES); $patientid = htmlentities($_POST['patientid_int'], ENT_QUOTES);

Il est possible de faire d'autres contrôles, comme le type de données attendu voir les méthodes is_*() ou ctype_*() pour cela.
Mais aussi la valeur attendue, un entier strictement positif par exemple, etc.

Code:

1 2 if (is_numeric($typinter) && $typinter > 0 ) { // traitement }
autant utiliser les fonction prevu pour eviter les injection SQL
Code:

1 2 3 4 $typinter = mysql_real_escape_string($_POST['typeinter_int']); $tec = mysql_real_escape_string($_POST['tec_int']); $patientid = mysql_real_escape_string($_POST['patientid_int']);
30/06/2008, 10h23
Telecaster

Merci !!! ;) Tous vos conseils m'aident à avancer ... Que du bon ... :yaisse2: