I know theres a better way to PHP/MySql this

design

#1

Hi. I was hoping I could get some help. I’m not very good with PHP or MySql, and I’m self taught, so my scripts are very ugly. This is my first time making a database driven website. If you read this snippet you’ll understand whats going on and what I’m trying to do. I know that I shouldnt be using a php switch: it’s not OOP. I just don’t know how to cross that gap.

<?php // $limit="20"; // Define them here? switch ($by) { case ("band"): $sql = "Select List.name, // all of this should be variable, but I can't make variables work here List.band, List.disk, List.cost, List.shipping, List.note From List Where List.cost <> 'null' // also want this to be user variable Order By // all of it should be vars... List.band Desc, Limit 20 // but don't know how. " ; $result = mysql_query($sql); if (!$result) { echo 'Band query failed, exiting.'; exit; } echo "Band order."; echo "\n"; echo <<<END END; break; case ("cost"): $sql = "Select require 'axe.php' // contains connect() index <?php ini_set('error_reporting', E_ALL); // connect() function connects us to the default database automagically. connect(); // supposed to list all data on the active table $by = ('band'); // using the switch while ($line = mysql_fetch_array($result, MYSQL_ASSOC)) { // filling rows and columns... echo "\t\n"; foreach ($line as $col_value) { echo "\t\t\n"; } echo "\t\n"; } // filled. echo "
Name Band Disk Cost Shipping Notes
$col_value
\n"; mysql_free_result($result); // something to do with performance // mysql_close($link); ?>

And there it is. The closest I can get to what I need without breaking everything is:

<?php $state = $_GET['State']; print($state); //to make sure you're getting what you wanted $query = SELECT * "; $query .= "FROM db "; $query .= "WHERE State = '" . $state . "' //the above method of coding the query is for neatness only print($query); //so that you can see the query //do the query and return errors if not right if(!($dbquery = mysql_query($query, $dbconnect))){ print("MySQL reports this error: " . mysql_error()); exit; } ?>

That code does what I am looking for (partially). It explains how to inject variables into SELECT… FROM… WHERE… but only three examples. What is “.=” What are the rules for ‘" "’ and “’ '” (single/double quote)? Somehow I feel that if the $_GET example had 4 variables instead of 3 I might wrap my head around it. I’ve been making no headway for 3 days straight. Someone please show me how easy this is. Thanks.
-John


#2

Not so sure why you was using the switch statement. So I removed it :P. You might have a few syntax errors when using it but you should be able to figure them out?
Interesting use of the sql and foreach loop later on. You might want to scrap that idea though. I think most people use select * unless there are lots of columns to get. And then instead of


#3

Heh, I had to check in another font whether that was a pipe | or the lowercase L. (Note to JeffT: this font sux) So if I wanted to make the entire query variable I would use:

$select = “l.name, l.band, ldisk”; // setting default values. Do with cookies in future versions.
$from = “List l”;
$acdc = “DESC”
$where = “l.cost <> NULL”
$ordby = “l.band” . $acdc; // does this come out as “l.band DESC” or not?
$limit = 20;

if(!empty($_GET[‘select’] && is_text($_GET[‘select’])){ // Im assuming that was ment to be a { not [
$select = $_GET[‘select’];
}
if(!empty($_GET[‘from’]) && is_text($_GET[‘from’])){ // and on down the list of variables…
$from = $_GET[‘from’];
}
if(!empty($_GET[‘acdc’]) && is_text($_GET[‘acdc’])){
$acdc = $_GET[‘acdc’];
}
// if(!empty($_GET[‘where’]) && is_something($_GET[‘limit’])){ // is what? Its got to pass operators and conditionals <> = >= etc.
// $where = $_GET[‘where’]; // Can I just tell it not to accept a text string longer than 10 or something? Security problem here?
// }
if(!ordby($_GET[‘ordby’]) && is_text($_GET[‘ordby’])){
$ordby = $_GET[‘ordby’];
}
if(!empty($_GET[‘limit’]) && is_numeric($_GET[‘limit’])){
$limit = $_GET[‘limit’];
} // done with variable declarations.

$sql = “SELECT $select”; //You could change this line to “SELECT *”
$sql .= " FROM $from"; //Assigning table name to shorter name for shorter sql statements
$sql .= " WHERE $where"; //If you put NULL in ‘’ then it treats it as string, it’s not
$sql .= " ORDER BY $ordby $acdc"; // <- is this gonna output right?
$sql .= " LIMIT $limit"; //Using the variable defined above

$result = mysql_query($sql);

Is this right? What am I missing besides the form? Thanks man.


#4

Why are you trying to use get variables for making the SQL?

Get variables (and post) should only be used for values that need to be changed at runtime. Things like getting product id, page numbers, order of display things. SELECT, FROM and other sql commands should not be seen by users.
As said above, you can use it for ordering things (passing ASC or DESC) but you should make sure that it is one of those values and if it isn’t, give it a default value.

You are right about security problems. If you are passing the sql data through GET, then anyone can change this information. Unless you have strict filtering techniques, someone would probably be able to insert ‘bad’ data through SQL injection techniques.
If you HAVE to pass the data through GET, then make sure you use the addslashes function.

This converts any single quotes to '. This stops it breaking any strings you have in SQL.

Example

someone could pass in the value of ';DELETE FROM band;– which would give an SQL statement of

This would actually delete all your data in that table. That’s why it’s a bad idea to do that.


#5

Well heres what the page should do.
I want the user to be able to change which values are shown after the mysql request. Like a sort by button on a form that changes what value everything is sorted by (name, price, etc).

How should I do it, knowing that $_GET is the least secure? Here is the page so far:

<?php require('axe.php'); // contains the connect() function which contains password and username. echo <<<HEAD index

Yo jo!

HEAD; if(!empty($_GET['select'])){ $select = $_GET['select']; } echo $select; if(!empty($_GET['from']) && is_text($_GET['from'])){ // and on down the list of variables.. $from = $_GET['from']; } if(!empty($_GET['acdc']) && is_text($_GET['acdc'])){ $acdc = $_GET['acdc']; } // if(!empty($_GET['where']) && is_something($_GET['limit'])){ // is what? Its got to pass operators and conditionals <> = >= etc. // $where = $_GET['where']; // Can I just tell it not to accept a text string longer than 10 or something? Security problem here? // } if(!empty($_GET['ordby']) && is_text($_GET['ordby'])){ $ordby = $_GET['ordby']; } if(!empty($_GET['limit']) && is_numeric($_GET['limit'])){ $limit = $_GET['limit']; } // done with variable declarations. else { $select = "l.name, l.band, l.disk"; // setting default values. Do with cookies in future versions. $from = "List l"; $acdc = "DESC"; $where = "l.cost <> NULL"; $ordby = "l.band " . $acdc; $limit = "20"; } ?> <?php $sql = "SELECT $select"; //You could change this line to "SELECT *" $sql .= " FROM $from"; //Assigning table name to shorter name for shorter sql statements $sql .= " WHERE $where"; //If you put NULL in '' then it treats it as string, it's not $sql .= " ORDER BY $ordby "; // <- is this gonna output right? $sql .= " LIMIT $limit"; //Using the variable defined above connect(); // resides in axe.php echo $sql; // For debugging only. echo '

'; $result = mysql_query($sql); if (!$result) { echo 'Query #1 failed, terminal abort. Please go back.
The database is not responding. Administration has been notified.'; exit; // Terminal error. } echo " Sorting database accoring to custom query."; echo "\n"; echo <<<END END; while ($line = mysql_fetch_array($result, MYSQL_ASSOC)) { echo "\t\n"; foreach ($line as $col_value) { echo "\t\t\n"; } echo "\t\n"; } echo "
Server Name Bandwidth Limit (GB) Disk Storage (MB) Price $/mo. Setup Fee (once) Editor Notes
$col_value
\n"; mysql_free_result($result); mysql_close($link); ?>// Form goes here, action is 'thisfile.php'

So far it outputs ‘Query #1 failed, terminal abort. Please go back.
The database is not responding. Administration has been notified.’. Also, any time I pass a variable in the url, the script ends after HEAD;

I’m not sure if it would work even if I gave $_GET a valid variable.