-5
$Con=mysql_connect('localhost', '849326_israel', '123456')
    or die('Problemas al seleccionar la base de datos');
mysql_select_db('pruebastest_zzl_registro', $Con)
    or die('Problemas db');
mysql_query("INSERT INTO RegUsuarios (Nombre, Email, PW) values
    ('$_REQUEST[Nombre]', '$_REQUEST[Email]', '$_REQUEST[PW]')",$Con) or die('Problema insertando datos');

It's saying...

Problemas insertando datos

I tried using $_POST and it doesn't work!

Thank you very much for helping me I found the error it was a typo

in my database I wrote Emal instead of Email so data could not be processed

tadman
  • 208,517
  • 23
  • 234
  • 262
Israel Jiménez
  • 21
  • 3

3 Answers3

3

xkcd

Now that that's out of the way, the error message is telling you that there's a problem with the insert query. I would like to suggest you try this:

$esc = "mysql_real_escape_string"; // for brevity
mysql_query($q="INSERT INTO `RegUsuarios` (`Nombre`,`Email`,`PW`) VALUES
    ('".$esc($_REQUEST['Nombre'])."','".$esc($_REQUEST['Email'])."',
    '".$esc($_REQUEST['PW'])."')");
if( $e = mysql_error()) {
  die("Error running the query:<br />".$q."<br />Error: ".$e);
}

This will dump out the exact query it was trying to run, as well as the exact MySQL error message. In an ideal situation you would log this to a file, instead of displaying it publically, but for development purposes this will do just fine. Note that I've added backticks ` around your table and column names - many people will tell you they're only needed if you name them something that's a reserved word, but personally I think it's just better to keep to the habit of putting them around all names: not only does it make it easier to visually identify such names (and allow colour-coding to work better), but it also helps avoid some of the more obscure keywords. Additionally, I've changed the or die to an actual if statement. This will allow you to do more than one thing, such as the logging to a file I mentioned.

Note that you don't need to pass the $Con variable around all the time - one of the main upsides of the mysql extension is that it will automatically assume the last opened connection.

Community
  • 1
  • 1
Niet the Dark Absol
  • 320,036
  • 81
  • 464
  • 592
  • Using `$esc` as short-hand for `mysql_real_escape_string` is probably a bad idea. Writing proper `mysql_query` code *should* look ugly because it is ugly. This is why an equivalent example in [PDO](http://net.tutsplus.com/tutorials/php/why-you-should-be-using-phps-pdo-for-database-access/) would be a lot easier to understand. – tadman Jul 11 '13 at 21:14
  • @tadman Because, of course, `new PDO("mysql:host=localhost;dbname=pruebastest_zzl_registro","849326_israel","123456")` is a LOT prettier than `mysql_connect('localhost','849326_israel','123456'); mysql_select_db('pruebastest_zzl_registro');`... /sarcasm – Niet the Dark Absol Jul 11 '13 at 21:17
  • You only connect once, but you will run hundreds of queries. PDO's `execute` with an array of bindings is about as simple as you can get. There isn't even a connect call in your answer, so I don't know why you'd bring that up. – tadman Jul 12 '13 at 14:46
  • And what about passing around that `$PDO` handler into functions and stuff? – Niet the Dark Absol Jul 12 '13 at 16:38
  • No harder than passing around your MySQL connection handle. Are you really protesting about this? `mysql_query` is a dead interface and it's super clunky to use even at the best of times. – tadman Jul 12 '13 at 17:52
  • But that's just it, you *don't* have to pass around a MySQL connection handle. I just don't see any reason not to use `mysql`, other than "most people are too stupid to know how to use it". – Niet the Dark Absol Jul 12 '13 at 18:17
  • You mean apart from the fact that it's [deprecated](http://php.net/manual/en/function.mysql-query.php) and being removed? Please, **DO NOT** encourage people to use it. Maybe it's served PHP well, but let it go. – tadman Jul 12 '13 at 20:16
1

There are several issues with your code.

First, you cannot simply call "$_REQUEST[Email]" inside a string like that. It will not get the value of the variable because there are brackets envolved. You have to use "{$_REQUEST[Email]}" instead.

Second, sinse the keys Email and Nombre are strings, its adequate that you properly declare them as such using "{$_REQUEST['Email']}".

In addition, its extremely dangerous to get values that come from the client-side and drop them in your SQL query directly with no filtering. You are exposing your site to SQL injection.

All and any data coming from client should be properly escaped with mysql_real_escape_string() before inserting it in your SQL query.

That being said, your code should be:

$nombre = mysql_real_escape_string($_REQUEST['Nombre']);
$email  = mysql_real_escape_string($_REQUEST['Email']);
$pw     = mysql_real_escape_string($_REQUEST['PW']);
mysql_query("INSERT INTO RegUsuarios (Nombre,Email,PW) values
    ('{$nombre}','{$email}','{$pw}')",$Con) or die('Problema insertando datos');

It is also a bad practice to store plain text passwords, and mysql_* functions are deprecated and will be soon removed from PHP. Consider storing the passwords as sha1() hashes, and migrate your code to use MySQLi.

Havenard
  • 27,022
  • 5
  • 36
  • 62
  • `"$_REQUEST[Email]"` is fine, that's how you interpolate array values. Not saying it's a good thing to do (I prefer concatenation), but it's not wrong. – Niet the Dark Absol Jul 11 '13 at 21:11
  • SHA1 can be cracked almost as easily as MD5, so it's best to use a [password specific encryption method](http://stackoverflow.com/questions/4795385/how-do-you-use-bcrypt-for-hashing-passwords-in-php). – tadman Jul 11 '13 at 21:12
0

You need quotes inside of the $_POST

$_REQUEST['Nombre']

"INSERT INTO RegUsuarios (Nombre,Email,PW) values
('".$_REQUEST['Nombre']."','".$_REQUEST['Email']."','".$_REQUEST['PW']."')"

As people are pointing out this code is open to SQL injection. Consider using PDO instead

Zevi Sternlicht
  • 5,399
  • 19
  • 31