0

I am currently in a condition not to find any errors in this line of code. I am trying to add several data to mysql database using PHP.

It gets "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 '', 'geo')' at line 5" Error message when I try to register someone. "geo" is the value of the radio group element.

<?php
if($_GET["regname"] && $_GET["regemail"] && $_GET["regpass1"] 
   && $_GET["regpass2"]&& $_GET["regfamilyname"] 
   && $_GET["reggivenname"] && $_GET["radioGroup"] )
{
    if($_GET["regpass1"]==$_GET["regpass2"])
    {
       $servername="localhost";
       $database="aaa";
       $username="bbbb";
       $password="cccc";

       $conn=  mysql_connect($servername,$username, $password) 
               or die(mysql_error());
       mysql_select_db($database,$conn);

       $sql="INSERT INTO users (username,password,familyname, 
             email, givenname, is_admin, created_time, category) values 
             ('" . $_GET["regname"] . "', '" .$_GET["regpass"]. "', '" 
             .$_GET["regfamilyname"]. "','" .$_GET["regemail"]. "', '" 
             .$_GET["reggivenname"]. "', '0', NOW()', '" 
             .$_GET["radioGroup"]. "')";

       $result=mysql_query($sql,$conn) or die(mysql_error());          
       print "<h1>You have registered sucessfully</h1>"; 
       print "<a href='index.php'>Go to login page</a>";
    }
    else print "Passwords doesnt match.";
}
    else print"Invalid data;";
?>
Yan Berk
  • 14,328
  • 9
  • 55
  • 52
Yagiz
  • 1,033
  • 2
  • 21
  • 47
  • You definitely read [this answer](http://stackoverflow.com/a/60496/569101) to avoid any SQL injetion ! – j0k Jul 14 '12 at 11:12
  • What's the error message ? How far does your script work (echo something somewhere to see if the "if" cases work). Echo the $sql to see the query... – Sliq Jul 14 '12 at 11:12
  • by the way, are you trying to build a login script ? There are some good ones out there... – Sliq Jul 14 '12 at 11:14
  • Yes it is an login page's php. There isn't any specific error message, but the error is inside the $sql – Yagiz Jul 14 '12 at 11:15

2 Answers2

0

Hard to say, because you haven't provided error message.

I guess error is here:

$sql="INSERT INTO users (id,username,password,familyname, email, givenname, is_admin, created_time, category) values ('','$_GET["regname"]', '$_GET["regpass"]', '$_GET["regfamilyname"]', '$_GET["regemail"]', '$_GET["reggivenname"]', '0', NOW()', '$radiobutton')";

You are inserting empty string into id column. Try fixing it, by removing id and '', there are also some quotes related problems:

$sql="INSERT INTO users (username,password,familyname, email, givenname, is_admin, created_time, category) values ('".$_GET["regname"]."', '".$_GET["regpass"]."', '".$_GET["regfamilyname"]."', '".$_GET["regemail"]."', '".$_GET["reggivenname"]."', '0', NOW(), '".$radiobutton."')";

This is probably not working because you have defined id filed as int PRIMARY KEY with AUTO_INCREMENT.

Also you are using old mysql_* functions, in a way wich leads to sql inejction vulnerabilities. You can use PDO with prepared statements.

UPDATE:

Zbigniew
  • 27,184
  • 6
  • 59
  • 66
  • id is an primary key. I want to increase the id number when I add a user. For example first users id will be 1, seconds will be 2. How can I do it? – Yagiz Jul 14 '12 at 11:14
  • NEW ERROR: 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 '', 'geo')' at line 5 – Yagiz Jul 14 '12 at 13:04
  • geo is the value of the radio group element – Yagiz Jul 14 '12 at 13:07
  • I had small typo (additional `'` near `NOW()`) so I've edited my post. I believe that this could be fixed by you. – Zbigniew Jul 14 '12 at 13:07
  • Also make sure that there is no `'` in any of your `$_GET` variables, like I said the way you write your script leads to sql injection vulnerabilities. – Zbigniew Jul 14 '12 at 13:08
0

The problem is that the query will break because all the terms like '$_GET["regname"]' which will break it using the ". You can use concatenation if you want to do it this way:

$sql="INSERT INTO users (username,password,familyname, 
email, givenname, is_admin, created_time, category) values 
('" . $_GET["regname"] . "', '" .$_GET["regpass"]. "', '" .$_GET["regfamilyname"]. "',
  '" .$_GET["regemail"]. "', '" .$_GET["reggivenname"]. "', '0', 
  NOW()', '" . $radiobutton. "')";


But a better practice will be using PDO.

An example:

$stmt = $dbh->prepare("INSERT INTO users (username,password,familyname, 
email, givenname, is_admin, created_time, category) values 
(:username,:password,:familyname,:email,:givenname,:is_admin,:created_time,:category)");

$taValues = array(
 'username'   =>  $_GET["regname"],
 'password'   =>  $_GET["regpass"],
 'familyname' =>  $_GET["regfamilyname"],
 'email'      =>  $_GET["regemail"],
 'givenname'  =>  $_GET["reggivenname"],
 'is_admin'   => '0',
 'created_time'  =>  NOW(),
 'category'      =>  $radiobutton
); 

PDOBindArray($stmt,$taValues);

$stmt->execute();
Nir Alfasi
  • 53,191
  • 11
  • 86
  • 129
  • NEW ERROR occurs when hitting register button: 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 '', 'geo')' at line 5 – Yagiz Jul 14 '12 at 13:04
  • geo is the radio button value – Yagiz Jul 14 '12 at 13:04
  • Without a dup of the variables it's hard to say. use `echo` or `print_r` to print the variable and print also the query. update your question with all this info and let's see what we can find. – Nir Alfasi Jul 14 '12 at 18:37