0

i don't know why but this is not working:

I am working on an Ajax send , i have send a value to a PHP script which parses the values and store the values in a php variable. and now:

$id = $_GET['id']; $url = $_GET['url']; $check = $_GET['check']; $filid = mysql_real_escape_string($id); $filurl = mysql_real_escape_string($url); if($check == 'true'){ $insert = "INSERT INTO tab (id,url) VALUES ('$filid','$filurl')"; mysql_query($insert) or die(mysql_error()); }

I am confirmed that my Ajax code is working , i'm not sure about php.When i remove the if statement and directly put the data into my database , it's working.

I'm new so please tolerate. I hope someone can point me out my mistakes are.

Thanks!

Improve this question
11
  • 4
    What does $_GET['check'] actually contain? Commented Aug 18, 2011 at 13:41
  • Add var_dump($_GET['check']); to see if you are getting 'true' Commented Aug 18, 2011 at 13:41
  • 2
    This code has a horrible security hole. Commented Aug 18, 2011 at 13:42
  • 2
    On a sidenote: use mysql_real_escape_string before you insert GET values into your tables. This code is prone to SQL injection: nl.php.net/manual/en/function.mysql-real-escape-string.php Commented Aug 18, 2011 at 13:43
  • 1
    You might also want to have a look at mysql_real_escape_string for the security sql injection issue Commented Aug 18, 2011 at 13:44

6 Answers 6

Reset to default
2

Please dear God! I can't believe that none of the answers so far have pointed out the massive SQL injection vector in the original code; to be honest it's better for the safety of your database that the if block isn't working as you originally intended!

You must sanatize, escape and clean up all data that comes from the user; failing to do this will leave your database wide open for SQL injection attacks. If this is new to you then I suggest you read up on some articles and possibly invest in a book on the subject.

PHP includes a function which will sanatize values for you called mysql_real_escape_string(), you should modify your SQL statement to read:

$clean_id = mysql_real_escape_string($id); $clean_url = mysql_real_escape_string($url); $insert = "INSERT INTO tab (id,url) VALUES ('$clean_id','$clean_url')";

To avoid having to remember such things I suggest you look into database abstraction layers which will handle this for you; MDB2 is a popular package although there are plenty of others out there such as doctrine.

Improve this answer
Sign up to request clarification or add additional context in comments.

3 Comments

Oh dear god. im not asking this. i know mysql_real_escape_string , i havent added it yet as im testing this code. and the values sent to store in the php variable is not input by users.
Seriously, regardless of testing it's vital that you sanitise all SQL statements, especially when posting code online where others will see it - lead by example.
Okay. i will edit it then. :( still dosen't solve my problem.
1

I think you probably want to check for true, rather than 'true' - you're looking for a string of text that happens to spell true, not the boolean true/false.

Improve this answer

1 Comment

Input variables coming from $_GET happen to be strings, always. Comparing against the boolean constant will only work by accident.
1

If $check is a boolean (or evaluates to a boolean value in comparisons):

if($check == true){

or better, just:

if ($check) {
Improve this answer

1 Comment

The === comparison is certain to fail, as the incoming variable is a string (or NULL).
0

Try changing the if statement to:

if ($check) {

What you want to do is evaluate it as a boolean, so you need to treat it as one. The sticking point here is what possible values $check can have as it comes from the client - the best thing to do it to have the client use 1 for true and 0 for false.

It's probably worth you reading this. Note that almost any string value (which is what you will get from $_GET and $_POST) will evaluate to true - the place you can easily get caught out here is the 'false' evaluates to true.

Improve this answer

Comments

0

Anything you get from the browser is iffy. There is nasty people about,

sanitize the inputs.

Improve this answer

Comments

0

We need to know what $check contains.. please provide more information. in general it will work:

if (!empty($check)) { $insert = "INSERT INTO tab (id,url) VALUES ('$id','$url')"; mysql_query($insert) or die(mysql_error()); }

if $check is a boolean just do like this:

if ( $check ) { $insert = "INSERT INTO tab (id,url) VALUES ('$id','$url')"; mysql_query($insert) or die(mysql_error()); }

and perheps it works:

if ( $check == TRUE ) {
Improve this answer

2 Comments

And what if $check is 'false'? :)
I had edited my answer. If 'true' or 'false' is a boolean verification in need, and forget my answer above..

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.