[RESOLU] Erreur sur un code php

bonjour,
j'ai eu l'erreur suivante d'afficher sur ma page localhost (wamp) :
Erreur SQL !
SELECT * FROM actualité WHERE id=
You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near '' at line 1.
Mon code est le suivant :

<?php
require "../config.php";
mysql_connect(DB_HOST,DB_LOGIN,DB_PASS);
mysql_select_db(DB_BDD);
if(!empty($_POST)){
extract($_POST);
$sql = "UPDATE actualité SET titre='$titre', contenu='$contenu' WHERE id=$id";
$req = mysql_query($sql) or die('Erreur SQL !<br/>'.$sql.'<br/>'.mysql_error());	
echo "actualité modifiée";
$_GET["id"]=$id;
}
$sql = "SELECT * FROM actualité WHERE id={$_GET["id"]}";
$req = mysql_query($sql) or die('Erreur SQL !<br/>'.$sql.'<br/>'.mysql_error());
$data=mysql_fetch_assoc($req);
?>

la ligne qui pose problème est la suivante :

$sql = "SELECT * FROM actualité WHERE id={$_GET["id"]}";
 

Je suis peut-être bête ou aveugle mais je n'arrive pas à voir ou est l'erreur
Quelqu'un voit pour moi
Modifié par Cadet (27 May 2010 - 10:41)

Salut Cadet
En regardant de plus près ta requête SQL, j'ai remarqué que tu as mis un accent dans le nom de ta table. L'encodage des caractères peut te péter assez facilement à la tronche...
Pareillement, je te conseillerais de remplacer

$sql = "SELECT * FROM actualité WHERE id={$_GET["id"]}";

par

$sql = "SELECT * FROM actualité WHERE id={$_GET['id']}";

Je pense que ça devrait sans doute mieux passer...

Merci pour ta réponse, je suis assez d'accord avec toi l'accent sur actulaité c'est pas forcément une bonne idée, je vais modifier cela.
Pour le code désolée mais j'ai déjà essayé ce que tu me proposes et j'ai le même message d'erreur !
c'est pourquoi je ne sais pas trop quoi faire d'autre.

salut

Le problème il est plutôt içi :

$_GET["id"] = $id;

get est un tableau de variables url, donc, ton assignation n'est pas correcte.

teste en faisant, par exemple, $id = 1;

http://fr2.php.net/manual/fr/reserved.variables.php

et puis attention à extract, c'est très dangeureux si tu ne maitrise pas, même si maitrisé d'ailleurs.

http://fr2.php.net/manual/fr/function.extract.php

à lire avec attention, parce que là ton code, c'est une passoire.

j'avais au départ le code suivant :

<?php
	require "../config.php";
	mysql_connect(DB_HOST,DB_LOGIN,DB_PASS);
	mysql_select_db(DB_BDD);
	extract($_POST);
	$sql = "SELECT * FROM actualité WHERE id={$_GET["id"]}";
	$req = mysql_query($sql) or die('Erreur SQL !<br/>'.$sql.'<br/>'.mysql_error());	
	$data=mysql_fetch_assoc($req);
?>

et même problème sur la ligne

$sql = "SELECT * FROM actualité WHERE id={$_GET["id"]}";

il n'y avait pas la ligne

$_GET["id"] = $id;

Eventuellement, tu peux essayer de construire ta requête en concaténant (un peu plus bourrin, mais ça devrait aller)

$sql = "SELECT * FROM actualité WHERE id=" . $_GET["id"];

Pourquoi tu t'embête avec des accolades et un extract() qui en plus d'être inutile, s'avère être dangereux dans ton code niveau sécurité ?

<?php

require "../config.php"; 

mysql_connect(DB_HOST,DB_LOGIN,DB_PASS); 
mysql_select_db(DB_BDD); 

if(!empty($_POST)) {
	
	// Ceci "remplace" le extract() par quelque chose de plus concret et plus sécurisé :
	$id = mysql_real_escape_string($_POST['id']);
	$titre = mysql_real_escape_string($_POST['titre']);
	$contenu = mysql_real_escape_string($_POST['contenu']);
	
	// Petit ajout pour la sécurité (si $id n'est pas un nombre) :
	if(!ctype_digit($id)) {
		die('Erreur!<br />Tentative d\'injection SQL!<br />');
	}
	
	$sql = "UPDATE `actualité` SET titre='". $titre ."', contenu='". $contenu ."' WHERE id='". $id ."'";
	$req = mysql_query($sql) or die('Erreur SQL !<br/>'.$sql.'<br/>'. mysql_error());  
	
	echo "actualité modifiée";
	
	$_GET["id"] = $id; 
}

// Petit ajout pour la sécurité (si $_GET['id'] n'est pas un nombre) :
if(!ctype_digit($_GET['id'])) {
	die('Erreur!<br />Tentative d\'injection SQL!<br />');
}

$sql = "SELECT * FROM `actualité` WHERE id='". mysql_real_escape_string($_GET['id']) ."'"; 
$req = mysql_query($sql) or die('Erreur SQL !<br/>'.$sql.'<br/>'. mysql_error()); 
$data = mysql_fetch_assoc($req); 

?>

Enjoy

Bonjour Khazou,
Merci pour ta proposition, j'ai essayé mais pas mieux, toujours la même phrase d'erreur.

bonjour jeff52,
Merci pour ton code, ok pour plus de sécurité, mais j'essayais déjà de faire marcher mon code seul. je ne suis pas toujours sur de ce que je fais...
J'ai donc appliqué ton code, mais j'ai ce message d'erreur qui apparait
Erreur!
Tentative d'injection SQL!
je précise que dans ma base, les id sont bien des nombres et à chaque news créee j'ai bien un chiffre en id qui s'est automatiquement crée. (news 1, news 3, la 2 a été supprimée donc de ce coté ça fonctionne....)
j'ai forcément loupé un truc, mais je n'arrive pas à savoir où. A force on ne voit plus rien.

Cadet a écrit :
bonjour jeff52,
Merci pour ton code, ok pour plus de sécurité, mais j'essayais déjà de faire marcher mon code seul. je ne suis pas toujours sur de ce que je fais...
J'ai donc appliqué ton code, mais j'ai ce message d'erreur qui apparait
Erreur!
Tentative d'injection SQL!
je précise que dans ma base, les id sont bien des nombres et à chaque news créee j'ai bien un chiffre en id qui s'est automatiquement crée. (news 1, news 3, la 2 a été supprimée donc de ce coté ça fonctionne....)
j'ai forcément loupé un truc, mais je n'arrive pas à savoir où. A force on ne voit plus rien.

Hmm est ce que cette page aura TOUJOURS un paramètre id= en GET (genre page.php?id=1) ? Ou bien elle peut être appelée seule (genre page.php) ?

Si on peut également l'appeler seule, alors remplace le second test conditionnel par :

<?php if(isset($_GET['id']) && !ctype_digit($_GET['id'])) ?>

Sinon, dans quel cas le message d'erreur apparaît t-il ? Quelle URL tente tu d'exécuter ?

Salut,

Que retourne cette ligne ? (À mettre avant la requête par exemple)

var_dump($_GET);

Au final le code pour regardé un article ce contente que d'une vérification de paramètre et une requête sql + affichage.

require "../config.php"; 

/**
 * @param string $query
 * @return resource|bool
 */
function query($query)
{
    $query = mysql_query($query) or exit('Erreur SQL !<br/>'.$query.'<br/>'.mysql_error());
    return $query;
}


mysql_connect(DB_HOST,DB_LOGIN,DB_PASS); 
mysql_select_db(DB_BDD);

var_dump($_GET);
//vérifie que id existe, le transforme entier et regarde si plus grand que 0 (un id est plus grand que 0)
//on peut aussi utilisé ctype_digit mais je trouve que convertir la variable est plus souple '^^
if (isset($_GET['id']) && ($_GET['id'] = (int)$_GET['id']) > 0)
{
    var_dump($_GET['id']);
    $data=mysql_fetch_assoc(query("SELECT * FROM actualité WHERE id={$_GET['id']}"));
    //on affiche le résultat... à l'arrache
    var_dump($data);
}

manipulations des type
intval ou (int)

@jeff52 : C'est mieux de faire les vérifications d'injection sql avant les real_escape_string. Et il est inutile de faire real_escape_string sur un nombre . Au passage, même si une page doit toujours être appeler avec un paramètre, il faut vérifié son existence et son type (string ou array), ne jamais faire confiance à l'utilisateur.

jo_link_noir a écrit :
@jeff52 : C'est mieux de faire les vérifications d'injection sql avant les real_escape_string. Et il est inutile de faire real_escape_string sur un nombre .

Pas faux, mais qui te dis que c'est forcément un nombre ?

jeff52 a écrit :

Pas faux, mais qui te dis que c'est forcément un nombre ?

la vérification faite avant xD

jo_link_noir a écrit :
la vérification faite avant xD

Ah d'accord, tu parles de la 2ème

En effet, je comprend ce que tu veux dire.. le fait de mettre $_GET['id'] = $id (qui a été vérifié) et ensuite refaire le test if(ctype_digit($_GET['id'])) , c'est ça ?

Moi je veux bien, mais je maintiens que cette 2ème vérification est nécessaire car l'assertion $_GET['id'] = $id ne se fait QUE si $_POST n'est pas vide.. Il suffirait au hacker de demander la page sans envoyer de données POST et de mettre un $_GET['id'] erroné pour contourner la première vérification.. seulement là, grâce au 2ème test, le $_GET['id'] est quand même vérifié

Ah non, je ne parlais pas de $_GET['id'] mais du contenu du premier if.

Il y a 3 mysql_real_escape_string et après une vérification pour savoir si $id est un nombre. Alors qu'on fessant la vérification avant on peut s'arrêter, pas besoin de protégé les variables ou faire la requête, de toute façon un paramètre n'est pas valide.
D'ailleurs faudrait aussi vérifier que id, titre et contenu existent. Ça retourne une erreur d'index non définie quand les niveaux d'erreur son au maximum (error_reporting()).

/** 
 * @param string $query 
 * @return resource|bool 
 */ 
function query($query) 
{ 
    $query = mysql_query($query) or exit('Erreur SQL !<br/>'.$query.'<br/>'.mysql_error()); 
    return $query; 
} 

//connexion à la db
//...

//sert de référence
$id = 0;

if(isset($_POST['id'], $_POST[''titre], $_POST['contenu'])) {
    //c'est ici, si id n'est pas un nombre on peut arrêter le traitement de l'update
    if(!ctype_digit($_POST['id'])) { 
        // Ça fait un code html bizarre en sortit [lol]. Le mieux d'enlever tout ces exit/die pour afficher des message plus explicite. Ou mieux, la liste des articles
        exit('Erreur!<br />Tentative d\'injection SQL!<br />');
    } 
    // Ceci "remplace" le extract() par quelque chose de plus concret et plus sécurisé : 
    $id = $_POST['id']; //id est un nombre pas besoin de le protéger
    $titre = mysql_real_escape_string($_POST['titre']);
    $contenu = mysql_real_escape_string($_POST['contenu']);
     
    if(query('UPDATE `actualité` SET titre="'. $titre .'", contenu="'. $contenu .'" WHERE id='. $id))
        echo 'actualité modifiée';
    else
        echo 'actualité non modifiée';
}

//ici $id faudra soit false, soit un nombre

//si $id est faux (0) et si $_GET['id'] est inexistant ou n'est pas un nombre ou est égal à 0, on le dit gentiment ^^.
if (!$id && (!isset($_GET['id']) || !ctype_digit($_GET['id']) || $id = $_GET['id']))
{
    echo 'aucun article sélectionné.';
}
else
{
    $req = query('SELECT * FROM `actualité` WHERE id='.$id);
    $data = mysql_fetch_assoc($req);
    var_dump($data);
}