1
$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

5
  • 3
    Ese error ocurre porque $_GET['id'] está vacío, entonces la sentencia SQL queda inconclusa. Commented el 23 mar. 2017 a las 12:35
  • 3
    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: php.net/manual/es/function.mysql-query.php y php.net/manual/es/security.database.sql-injection.php
    – A. Cedano
    Commented el 23 mar. 2017 a las 12:40
  • 2
    Nunca utilices entradas sin sanear en una consulta a tu base de datos, hacen tu código vulnerable a ataques de inyección SQL
    – Alvaro Montoro
    Commented el 23 mar. 2017 a las 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. php.net/manual/es/pdo.prepared-statements.php
    – A. Cedano
    Commented el 23 mar. 2017 a las 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
    – oscar
    Commented el 24 mar. 2017 a las 8:58

1 respuesta 1

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();

    }

}
7
  • 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
    Commented el 23 mar. 2017 a las 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
    Commented el 23 mar. 2017 a las 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
    Commented el 23 mar. 2017 a las 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
    Commented el 23 mar. 2017 a las 12:58
  • 1
    Vale, voy a añadir una sección final mostrando cómo debiese ser el código usando PDO
    – ffflabs
    Commented el 23 mar. 2017 a las 13:17

Tu Respuesta

By clicking “Publica tu respuesta”, you agree to our terms of service and acknowledge you have read our privacy policy.

¿No es la respuesta que buscas? Examina otras preguntas con la etiqueta o formula tu propia pregunta.