2

I'm trying to make like few text boxes and upon submit another php will be connected let's say it's database.php then the database.php will insert what's in the textboxes into the mysql database table but I'm not sure how I should be into the mysqli_query code....what I have now is

mysqli_query($db,"INSERT INTO jliu VALUE(null,$_GET['title'],$_GET['fname'],$_GET['lname'],$_GET['description'])");

and of course the $_GET won't work properly. I can't really figure how to get it correctly.

Dharman
  • 30,962
  • 25
  • 85
  • 135
Dora
  • 6,776
  • 14
  • 51
  • 99
  • 2
    Never use the $_GET variables directly in to the query due to security reason. – Prasanth Bendra Mar 26 '13 at 09:36
  • 10
    never, never ever(!!!) directly insert `$_GET` into your mysql querys! – simplyray Mar 26 '13 at 09:37
  • +1000 for the previous comments. Also: did you try dumping `$_GET` to see what is defined? Any error in the logs? – romainberger Mar 26 '13 at 09:37
  • I recommend this article: [Getting Clean With PHP](http://net.tutsplus.com/tutorials/php/getting-clean-with-php/). If you are using some PHP framework, most probably it has input sanitizing methods – piotr_cz Mar 26 '13 at 09:40
  • ohoh...I know because $_GET is like the worst to use but I'm just starting to learn php and this is just some work I'm looking at the book and it's asking me to do so ya hehe....thanks thanks for reminding ^_^ – Dora Mar 26 '13 at 09:41
  • Consider using a class like Zend_DB (Doctrine 2 is probably too much for your purpose). It's easy to use without the whole Zend Framework stack and will help you avoiding severe security mistakes. There's a short [example](http://stackoverflow.com/questions/4840941/zend-db-without-zend-framework) – herrjeh42 Mar 26 '13 at 09:43
  • $_GET is not the worst to use. . .what you create with that is a free user interface for the user to alter your database, it's called a sql injection vulnerability, the server will execute any sql the user enters in those textfields on your database. $_GET variables are generally used when the user searches your website and you would do this search with a database user that only has read rights to the db, but you have to sanitize the variable before inserting it into a query – pythonian29033 Mar 26 '13 at 09:45
  • Why do you not make query string with sprintf function, It can make you string as you want. – Dieepak Mar 26 '13 at 10:16

7 Answers7

2

You need to break out of the string in order to concatenate it with the $_GET variables.

mysqli_query($db,"INSERT INTO jliu VALUES(null,".$_GET['title'].",".$_GET['fname'].",".$_GET['lname'].",".$_GET['description'].")");

But really you should look into prepared statements to avoid the gaping SQL injection security hole above.

With prepared statements you would be binding the $_GET variables to the parameters in the query.

$stmt = mysqli_prepare($db,"INSERT INTO jliu VALUES(null, ?, ?, ?, ?");

mysqli_stmt_bind_param($stmt, "s", $_GET['title']); 
...
// and the same for the others

There's some more detail in the linked manual page on how to execute the prepared statement and return the result.

bcmcfc
  • 25,966
  • 29
  • 109
  • 181
  • ah~ I'm so so so stupid....I did what you have here but then I just somehow my head isn't spinning...the last ) was what made me think what should I put to get the closing bracket and omg I could just use ")" then ya....let me try it out...omg....another stupid question I asked – Dora Mar 26 '13 at 09:40
  • Not as such... if it leads you to using prepared statements instead then it's a good job you asked it ;-) – bcmcfc Mar 26 '13 at 09:43
  • thanks for the prepared statements. I kind of looked at the php manual but guess I'm not too good with php yet their examples makes my brain spin but yours seem to easy to understand and I will try that out after I tried what I started ^_^ – Dora Mar 26 '13 at 09:44
2

Please don't use mysql_query() and consorts, they are way too unsafe. Do have a look at PDO and use that instead. Specifically PDO::prepare() and PDOStatement::execute() will be your solution.

theintz
  • 1,946
  • 1
  • 13
  • 21
2

Your actual issue, inserting index values of an array into a string can be solved in two ways (more if you count heredoc, sprintf and whatnot):

Use curly braces:

"INSERT INTO jliu VALUE(null, {$_GET['title']}, {$_GET['fname']}, {$_GET['lname']}, {$_GET['description']})"

Use concatenation:

"INSERT INTO jliu VALUE(null, " . $_GET['title'] . ", " . $_GET['fname'] . ", " . $_GET['lname'] . ", " . $_GET['description'] . ")"

However, the overriding issue is that you shouldn't be doing this. With your code, you're wide open to SQL Injection vulnerabilities.

Use prepared statements, and insert your values in there. This creates a more secure (and marginally more performant) query, as you're only binding the values, and not changing the actual query itself. Code example:

if ($stmt = $mysqli->prepare("INSERT INTO jliu VALUES (NULL, ?, ?, ?, ?)")) {
    $stmt->bind_param("ssss", $_GET['title'], $_GET['fname'], $_GET['lname'], $_GET['description']);

    $stmt->execute();
}
Rudi Visser
  • 21,350
  • 5
  • 71
  • 97
1

You have to use a prepared statement and replace the variables with a placeholder.

http://php.net/manual/de/mysqli.prepare.php

René Höhle
  • 26,716
  • 22
  • 73
  • 82
-2

try this..

    $title = mysqli_real_escape_string($_GET['title']);
    $fname = mysqli_real_escape_string($_GET['fname']);
    $lname = mysqli_real_escape_string($_GET['lname']);
    $description = mysqli_real_escape_string($_GET['description']);
    $stmt = mysqli_prepare($db,"INSERT INTO jliu VALUES(null,'".$title."','".$fname."','".$lname."','".$description."'");
    mysqli_stmt_execute($stmt);
MKV
  • 913
  • 7
  • 6
-2

Answering your question:

You may do something like below. Actually, that is to put a variable of array item into a string, not only for $_GET. Any array may be accessed like that:

$str = "some text {$_GET['title']}"

Or similar approach when you are working with objects:

$str = "some text {$obj->test}"

But, that is bad idea when you are working with sql queries. You should use some functions to clear your input in order to prevent sql injections.

The most stratiforward way is to use mysqli_real_escape_string:

$str = "some text " . mysqli_real_escape_string($_GET['title'])." ...";

Or you can use prepared statements which are more flexible

Viktor S.
  • 12,736
  • 1
  • 27
  • 52
-2

It should be like this.

$query = sprintf("INSERT INTO jliu VALUE(null,%s,%s,%s,%s)",$_GET['title'],$_GET['fname'],$_GET['lname'],$_GET['description']);
mysqli_query($db,$query);
Dieepak
  • 546
  • 2
  • 7