Click to See Complete Forum and Search --> : Very, very, very obvious SQL code.


onion2k
11-30-2004, 07:46 PM
$dbfield['first_name'] = "'".addslashes($_POST['firstname'])."'";
$dbfield['last_name'] = "'".addslashes($_POST['lastname'])."'";
$dbfield['email_address'] = "'".addslashes($_POST['email'])."'";

if ($_POST['id'] == 0) {

$sql = "insert into table (";
$sql .= implode(",",array_keys($dbfield));
$sql .= ") values (";
$sql .= implode(",",array_values($dbfield));
$sql .= ")";

} else {

foreach ($dbfield as $field => $value) {
$sqlarray[] = $field." = ".$value;
}

$sql = "update table set ";
$sql .= implode(", ",$sqlarray);
$sql .= " where id = '".$_POST['id']."'";

}

echo $sql;


Instead of building huge long strings of SQL just put everything into an associative array with the keys as the database column names, and the values as the form data.. and a little code at the end generates your SQL string. Magic.

I've been doing this sort of thing for agggggges, but the number of posts in other folders with great long SQL variables leads me to believe others might not have figured out the lazy way.. So I'm letting the cat out of the proverbial bag.

Aren't I nice?

EDIT: I know the code is a bit longer for 3 form fields than building a string.. just imagine you have 100 fields though..

Shrike
12-01-2004, 04:36 AM
/*
$table descriptions is the result of a describe query for each table you need to update
e.g. tablename = array (column1, column2), tablename2 = array(...) etc
$table_ids is a list of the primary key values for the above tables
Code assumes the primary key is first in each table description
It could probably use a filter to prevent certain columns being updated : )
*/
foreach($table_descriptions as $tablename => $table){
foreach($table as $column){
if(array_key_exists($column, $_POST)){
$sqllist[] = "$column = '".$_POST[$column]"'";
}
}
$sqls[$tablename] = $$tablename = "
UPDATE $tablename
SET ".implode(",\n", $sqllist)."
WHERE ".$table[0]." = ".$table_ids[$table[0]];
$sqllist = array();
}
foreach($sqls as $sql){
$result = $mysql_query($sql);
}

I wanted to write a class which could generate selects, inserts and updates across multiple tables, but it proved quite difficult so I never bothered :)

onion2k
12-01-2004, 05:12 AM
Thats nice, but you end up using database column names as form names, which, in my opinion, is a security risk. Its a big clue for SQL injection attacks.

I can't believe how many commas there are in that first sentence.

Shrike
12-01-2004, 05:37 AM
True, to an extent. Although SQL injections are possible regardless, if you assume that an HTML form field does in fact relate directly to a database column, even if they are named differently.

To be absolutely sure you could just do what you have done, but perhaps in a more generic way, e.g.

$inputoutput = array(
"htmlfield" => "databasecolumn",
"htmlfield2" => "databasecolumn2"
);
foreach($_POST as $key => $value)
{
$input[$key] = $inputoutput[$key];
}

In any case, as long as you validate thoroughly, SQL injections should never be an issue :)

PHPMagician
12-03-2004, 08:30 AM
<?php
function generate_sql_query($query,$names,$values)
{
foreach ($names as $key => $value) {
// If the value exists within values.
if (isset($values[$value])) {
$dbvalues[]=(get_magic_quotes_gpc())?'\''. $values[$value] .'\'':'\''. addslashes($values[$value]) .'\'';
}
else {
return false;
}
$dbfields[] = (is_numeric($key))?$value:$key;
}

$dbfields = implode(',', $dbfields);
$dbvalues = implode(',', $dbvalues);

$sql = sprintf($query,$dbfields,$dbvalues);
return $sql;
}

$names = array('username','ccnumber_field' => 'ccnumber');
echo generate_sql_query('INSERT INTO tablename (%s) VALUES (%s)',$names,$_POST);
?>


Kindof a bit of both.

If the form name and the field name are the same, just use an integer key (in other words, don't make it associative). If they share different names, the key should be the databases field name and the value should be the forms name.

In my example, username is both the inputs name and fieldname, while ccnumber_field is the fieldname and ccnumber is the inputs name.

This, to me, seems more automatic and less lines of code to write (instead of writing addslashes manually for everyone and allows you to have field names and input names different.