I know theres a better way to PHP/MySql this...

I know theres a better way to PHP/MySql this...

Posted by: ddefesse
Posted on: 2005-10-18 22:22:00

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 "<table>n";

echo <<<END
<tr>
<td>Name</td>
<td>Band</td>
<td>Disk</td>
<td>Cost</td>
<td>Shipping</td>
<td>Notes</td>
</tr>
END;
break;

case ("cost"):
$sql = "Select


require 'axe.php' // contains connect()
<head><meta http-equiv="content-type" content="text/html;charset=utf-8" />
<title>index</title>
</head>
<body>
<?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<tr>n";
foreach ($line as $col_value) {
echo "tt<td>$col_value</td>n";
}
echo "t</tr>n";
} // filled.
echo "</table>n";
mysql_free_result($result); // something to do with performance
// mysql_close($link);

?>
</body>

</html>

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

Re: I know theres a better way to PHP/MySql this..

Posted by: decswxaqz
Posted on: 2005-10-19 03:06:00

In reply to:


<?php
ini_set('error_reporting', E_ALL);

//You should generally put all your requires and includes at the top
//That is why your queryies won't work, because it's not connecting first
require('axe.php');

//Have to use connect function?
connect();

$limit = 20;
//I am guessing you want to specify a limit of how many results to get?
//If so you can uncomment the next few lines and then pass the limit through the URL

//if(!empty($_GET['limit']) && is_numeric($_GET['limit']))[
// $limit = $_GET['limit'];
//}


$sql = "SELECT l.name,l.band,l.disk,l.cost,l.shipping,l.note"; //You could change this line to "SELECT *"
$sql .= " FROM List l"; //Assigning table name to shorter name for shorter sql statements
$sql .= " WHERE l.cost <> NULL"; //If you put NULL in '' then it treats it as string, it's not
$sql .= " ORDER BY l.band DESC";
$sql .= " LIMIT $limit"; //Using the variable defined above

$result = mysql_query($sql);
if (!$result) {
echo 'Band query failed, exiting.';
exit;
}
?>
<head><meta http-equiv="content-type" content="text/html;charset=utf-8" />
<title>index</title>
</head>
<body>
Band order<br/>
<table>
<tr>
<td>Name</td>
<td>Band</td>
<td>Disk</td>
<td>Cost</td>
<td>Shipping</td>
<td>Notes</td>
</tr>

<?php
while ($line = mysql_fetch_array($result, MYSQL_ASSOC)) {
echo "t<tr>n";
foreach ($line as $col_value) {
echo "tt<td>$col_value</td>n";
}
echo "t</tr>n";
}
echo "</table>n";
mysql_free_result($result); // something to do with performance
// mysql_close($link);

?>
</body>
</html>



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

In reply to:


while ($line = mysql_fetch_array($result, MYSQL_ASSOC)) {
echo "t<tr>n";
foreach ($line as $col_value) {


use

In reply to:


while ($line = mysql_fetch_array($result, MYSQL_ASSOC)) {
echo "t<tr>n";
$band = $line['band'];
...
echo "tt<td>$band</td>n";


If you was to add another column to the sql statement but forgot to change anything else, it would mess up your layout. Just my opinion though.

Re: I know theres a better way to PHP/MySql this..

Posted by: ddefesse
Posted on: 2005-10-19 19:10:00

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.

Re: I know theres a better way to PHP/MySql this..

Posted by: decswxaqz
Posted on: 2005-10-20 00:19:00

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.

In reply to:

addslashes($_GET['select']);


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

Example

In reply to:

$sql = "SELECT * FROM List WHERE l.band = '".$_GET['band']."'";


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

In reply to:

$sql = "SELECT * FROM List WHERE l.band = '';DELETE FROM band;--' ";


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

Re: I know theres a better way to PHP/MySql this..

Posted by: ddefesse
Posted on: 2005-10-20 00:41:00

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:

<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd">
<?php
require('axe.php'); // contains the connect() function which contains password and username.
echo <<<HEAD
<html xmlns="http://www.w3.org/1999/xhtml">
<head><meta http-equiv="content-type" content="text/html;charset=utf-8" />
<title>index</title>
</head>

<body>
<p>Yo jo!</p>
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 '<br /><br />';
$result = mysql_query($sql);
if (!$result) {
echo 'Query #1 failed, terminal abort. Please go back. <br /> The database is not responding. Administration has been notified.';
exit; // Terminal error.
}

echo " Sorting database accoring to custom query.";
echo "<table>n";
echo <<<END
<tr>
<td>Server Name</td>
<td>Bandwidth Limit (GB)</td>
<td>Disk Storage (MB)</td>
<td>Price $/mo.</td>
<td>Setup Fee (once)</td>
<td>Editor Notes</td>
</tr>
END;
while ($line = mysql_fetch_array($result, MYSQL_ASSOC)) {
echo "t<tr>n";
foreach ($line as $col_value) {
echo "tt<td>$col_value</td>n";
}
echo "t</tr>n";
}

echo "</table>n";


mysql_free_result($result);
mysql_close($link);

?>// Form goes here, action is 'thisfile.php'
</body>
</html>

Tags: php mysqlmy first timegapsnippetoopuglyhopingscriptscrosshelp