Error en consulta mysql con php

Question

$re=mysql_query(
                "select * from productos where id=".$_GET['id']
                )
                or die (mysql_error());

En esta línea superior me da este error, pero no consigo encontrarlo:

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

Ese error ocurre porque $_GET['id'] está vacío, entonces la sentencia SQL queda inconclusa. — javiertapia, Mar 23 '17 at 12:35
Hay un error **peor**: estás usando `mysql_query`, una **función obsoleta**. _Esta extensión fue declarada obsoleta en PHP 5.5.0 y eliminada en PHP 7.0.0. En su lugar debería utilzarse las extensiones MySQLi o PDO_MySQL_ Esa forma de usar la BD es presa fácil de inyección SQL. Ver: http://php.net/manual/es/function.mysql-query.php y http://php.net/manual/es/security.database.sql-injection.php — A. Cedano, Mar 23 '17 at 12:40
Nunca utilices entradas sin sanear en una consulta a tu base de datos, hacen tu código vulnerable a ataques de [inyección SQL](http://es.stackoverflow.com/q/10518/250) — Alvaro Montoro, Mar 23 '17 at 12:40
Un ejemplo de **inyección SQL**: Si tu `$_GET['id']` es tomado de un input de formulario y cualquier usuario pone esto: `1; DELETE FROM unatabla; DELETE FROM otratabla; DELETE FROM oootratabla;` se podrían enviar a tu BD varias consultas muy maliciosas, desde borrar tablas hasta obtener la contraseña del administrador o alterar datos. Se recomienda por eso usar sentencias preparadas mediante PDO o MySQLi para evitar dolores de cabeza. http://php.net/manual/es/pdo.prepared-statements.php — A. Cedano, Mar 23 '17 at 12:55
entonces, que podria poner para que fuera bien, soy un poco ,bastante novato y voy algo perdido en el tema de carritos de compra, decir que id no esta vacio, tiene productos — racxo, Mar 24 '17 at 08:58

ffflabs · Answer 1 · 2017-03-24T09:44:19.727

7

Tu código tiene varios problemas:

Estás usando interpolación directa de variables en una sentencia
Estás usando un conector obsoleto
Estás usando directamente una variable superglobal sin comprobar si existe
Estás utilizando die() en vez de excepciones

Sin embargo, todo eso va por el lado de las buenas prácticas. Asumiendo que no tienes intención de cambiar tus prácticas, el error puntual se puede debuggear haciendo:

$sentencia="select * from productos where id=".$_GET['id'];
print_r($sentencia);
$re=mysql_query($sentencia)  or die (mysql_error());

Eso no soluciona tu problema, pero es el camino para que tú mismo puedas diagnosticar qué le estás pasando a mysql_query en vez de simplemente asumir que "no encuentras el error".

Recomendación

No uses el conector php_mysql. Usa PDO o MySQLi. Usa sentencias preparadas en vez de interpolación directa de variables. Comprueba la existencia de una superglobal en vez de usarla directamente. Usa excepciones en vez de die().

Cómo quedaría con PDO:

if(!isset($_GET['id']) ){ // <-- compruebo la existencia de la superglobal

    echo 'No está fijado el parámetro "id"';

} else {

    $id = $_GET['id'];

    $sentencia="select * from productos where id=:id";

    $stmt = $conn->prepare($sentencia); // <-- uso una sentencia preparada

    try {

        $stmt->execute([':id'=>$id]); // <-- PDO sanitiza el parámetro $id

    } catch (\PDOException $e) {

        echo 'Ocurrió un error en la consulta: '.$e->getMessage();

    }

}

edited Mar 24 '17 at 09:44

answered Mar 23 '17 at 12:45

ffflabs

21,223
25
48

2

Muchas veces "no encuentro el error" significa "me ha salido este error, he mirado por encima, no he buscado nada y que alguien me lo encuentre" (Racxo, no digo que sea tu caso :P) – lois6b Mar 23 '17 at 12:48
_Estás usando un conector obsoleto_ cuya consecuencia podría ser la **inyección SQL**. Si el $_GET['id'] es tomado de un formulario y cualquiera pone en el campo id lo siguiente: `1; DELETE FROM unatabla` se mandan dos consultas a la BD, una con el `SELECT` y otra que borra una tabla. Y si quiere, borra todas las tablas de la BD: `1; DELETE FROM unatabla; DELETE FROM otratabla; DELETE FROM oootratabla;` Yo no indicaría soluciones al OP sobre un código altamente riesgoso. Saludos. – A. Cedano Mar 23 '17 at 12:50
En realidad es una respuesta testimonial. Acepto que debiera mostrar cómo hacer el mismo código usando mysqli o PDO, pero si el OP no se molestó en debuggear por sí mismo, no creo que se moleste en cambiar todos sus usos del driver antiguo – ffflabs Mar 23 '17 at 12:52
Creo que darle una solución es ayudarle a que siga perpetuando su error, hasta que se encuentre con la desagradable sorpresa de una inyección SQL. Y por otra parte, la _aparente solución del problema_ podría ser copiada por otro usuario que igual que él no se ha siquiera dado cuenta de que hay otras formas más seguras de acceder a los datos. – A. Cedano Mar 23 '17 at 12:58
1

Vale, voy a añadir una sección final mostrando cómo debiese ser el código usando PDO – ffflabs Mar 23 '17 at 13:17
gracias amenadiel, he metido el codigo pdo, pero me da error en la primera llave del if. – racxo Mar 24 '17 at 09:16
Faltaba un paréntesis, pero igual tienes que cambiar tu conexión para usar PDO. De otra manera tu objeto $conn no tendrá la forma esperada – ffflabs Mar 24 '17 at 09:46

Error en consulta mysql con php

1 Answers1