Help with syntax T_IF problem

apower

Hi guys,

Ive been experimenting with the INSERT INTO command and am having a few problems with my code. The SQL statement looks good and if filled with all the correct details but every time i think i know what the problem is i come to a dead end again. The code is as follows

<?php

$con = mysql_connect("local host","user","pass");

if (!$con) 
	{	
	die('Could not connect' . mysql_error());
	}

mysql_select_db("my_db", $con);

$SQL="INSERT INTO GIFT (`title`, `desc`, `gender`, `event`, `age`, `imglnk`, `productlnk`, `price`) 

VALUES 
('$_POST[title]', '$_POST[desc]', '$_POST[gender]', '$_POST[occasion]','$_POST[age]', '$_POST[imagelink]', '$_POST[productlink]', '$_POST[price]')"

if (!mysql_query($SQL,$con))
   {
die('Error: ' . mysql_error());
	}

echo "1 record added";

mysql_close($con)

?>

The error is syntax error, unexpected T_IF on line 17 which is where i run the mysql query "if (!mysql_query($SQL,$con))"

thanks again

Weedpacket

Because you forgot the semicolon on the end of the previous statement.

johanafm

Also escape any user input before using it in a query. As it is now, the way you use input data makes you vulnerable to SQL injection. Cast numbers to floats and ints, and make strings safe with mysql::real_escape_string.

Derokorian

Step further: Don't use the mysql extension but upgrade to mysqli or PDO. The mysql_* based function are on their way out, the i in mysqli stands for improved.

Quote from PHP.net

It is recommended to use either the mysqli or PDO_MySQL extensions. It is not recommended to use the old mysql extension for new development.

mrjoeblack

guys, am interested in this post.... I'm still trying to get my head around some of this security stuff...

Johanafm... you said

Cast numbers to floats and ints, and make strings safe with mysql::real_escape_string

Can you elaborate or explain this better, to see if my brain get's it.

Derokorian... you said

Don't use the mysql extension but upgrade to mysqli or PDO. The mysql_* based function are on their way out, the i in mysqli stands for improved

I'm not clear what part you are replacing, can you explain this better for an idiot like me.

thanks guys 🙂

Derokorian

Basically to use mysqli instead of mysql (which is the easier change) every call to mysql_query would become mysqli_query, every mysql_connect would be mysqli_connect, etc etc.

bradgrafelman

mrjoeblack;10996656 wrote:
Can you elaborate or explain this better, to see if my brain get's it.

As the function name suggests, [man]mysql_real_escape_string/man is only good for sanitizing string data - using it on numerical data (integers, floats, etc.) would be inappropriate.

Instead, you should either verify that the numerical data is actually numerical (e.g. if you expect $_GET['id'] to be an integer ID value, make sure it only contains numbers and not something like "123 OR 1 == 1") or simply "cast" the string data to the appropriate numerical type. See [man]types.type-juggling[/man] for more information on type casting and the like. PHP also has other validation functions for numerical data, such as [man]is_numeric/man, [man]ctype_digit/man, etc.

Looking at it from the SQL side, note that if a column is one of the numerical types, its values should not be enclosed with quotes; '3' is a string containing the ASCII character '3', and 3 is the number that comes after two.

So, if you've got data that you expect to be numerical, don't surround it with quotes and do sanitize/validate it as a numerical value.

mrjoeblack;10996656 wrote:
I'm not clear what part you are replacing, can you explain this better for an idiot like me.

Any function that begins with "mysql_" is from the [man]mysql[/man] extension, an extension which is outdated and no longer being actively developed. In other words, the PHP developers have abandoned it and recommend that everyone else does too.

It has been superseded some time ago by the [man]MySQLi[/man] extension (or perhaps even [man]PDO[/man] if you're looking for a more abstract interface).

I know it's full of a lot of information, but consider taking a look at the Overview of the MySQL PHP drivers section of the PHP manual (especially the 'Choosing an API' section: [man]mysqlinfo.api.choosing[/man]).

mrjoeblack

It has been superseded some time ago by the MySQLi extension

so is it just a case of going through the script and adding an "i" onto all the mysql script, like change mysql to mysqli_ ????

Derokorian

Basically, yes.

bradgrafelman

Derokorian;10996669 wrote:
Basically, yes.

I'd have to disagree. While the two extensions do behave remarkably similar, there are some distinct differences.

For example, [man]mysqli_query/man requires at least 2 parameters, while [man]mysql_query/man only requires 1.

Bonesnap

mrjoeblack;10996667 wrote:
so is it just a case of going through the script and adding an "i" onto all the mysql script, like change mysql to mysqli_ ????

Yes, but it's not quite that simple. For example, mysql_query() doesn't require a resource link (it's optional), whereas mysqli_query() does require a resource link.

Furthermore, the arguments are reversed: mysql_query() takes the query as its first argument and the resource as its second (optional) argument, whereas mysqli_query() takes the resource as its first argument and the query as its second argument. Just be careful going through your code and arbitrarily adding an "i" to your MySQL commands.

EDIT: brad is faster than me. 🙁

Derokorian

Ugh, right forgot about the resource link. I always use the OOP version of Mysqli so the resource identifier would be the object itself. Sorry about that, did not mean to give you misinformation.

Weedpacket

bradgrafelman;10996663 wrote:
As the function name suggests, [man]mysql_real_escape_string/man is only good for sanitizing string data - using it on numerical data (integers, floats, etc.) would be inappropriate.

Instead, you should either verify that the numerical data is actually numerical...[and so on and so forth]

Or just take the other part of the advice, stop using the mysql extension, and use the prepared statement functionality provided by the both of the two alternatives.

bradgrafelman

Weedpacket;10996679 wrote:
Or just take the other part of the advice, stop using the mysql extension, and use the prepared statement functionality provided by the both of the two alternatives.

True, that would solve the issue of properly sanitizing the data, but it might still be desirable to do some verification first.

For example, if you're asking for my salary and expect it to be in DECIMAL format but I instead say '$60k', it might be more helpful to do check if the data was numerical and, if not, skip the SQL query altogether and output an error message informing me of the proper format.

mrjoeblack

boy you guys kill me man, you guys kill me! Am I supposed to actually follow that stuff hahahaha!!. the time it takes you to write your professor type stuff, you could have just sorted it for me hahaha!!

I know, I know, I'm supposed to be learning here, not just getting things done the lazy way, right?? I have learned that much, only it doesn't get the job done so quickly is the problem. We're needing to build an empire here, we dont have time to learn sql to an nth degree hahahah!!

(((maybe if i sent whisky over, that would help!! he thinks 😕)))

Weedpacket

bradgrafelman wrote:
For example, if you're asking for my salary and expect it to be in DECIMAL format but I instead say '$60k', it might be more helpful to do check if the data was numerical and, if not, skip the SQL query altogether and output an error message informing me of the proper format.

I agree; I think there's a tendency to conflate "verification" with "sanitising". It's not helped by the habit of many people in these forums of using the "mysql_query() or die()" idiom (also known as the Soup Nazi school of customer service, or the "making decisions is harder than shooting myself in the head" idiom, or ....). If there's no attempt to recover from user errors then the verification process begins and ends with sanitisation.