Re: [PHP] Code Review PLEASE !!!

Jordan S. Jones Mon, 05 Apr 2004 15:57:14 -0700

Wells first of all, you are going to want better form input validation.
For Example:

foreach ($_POST['fleet_id'] as $key => $value) {
   $fleetCode = $_POST['fleet_code'][$key];
   $historyUrl = $_POST['history_url'][$key];
   $downloadUrl = $_POST['download_url'][$key];
   mysql_query("UPDATE imp_fleet SET fleet_code = '$fleetCode', history_url = 
'$historyUrl', download_url = '$downloadUrl' WHERE fleet_id = $value") or die 
(mysql_error());
 }

Are you sure that $_POST['fleet_id'] is valid? or even a number?

What happens with $_POST['fleet_id'] == '1 = 1'?? Well, long story short, imp_fleet has no more records.

Just a simple example of a huge problem.

Jordan S. Jones

Matthew Oatham wrote:

Hi,

I am a newbie PHP programmer, I have some code that works but I want some tips on how I an Improve my code, i.e. should I be doing my updates / deletes on same php page as the display page, am I using transactions correctly, am I capturing SQL errors correctly am I handling form data as efficient as possible?

My code displays some information from a database and gives users the chance to delete or edit any field and is as follows:

<?

include ("../db.php");

$acton = $_POST['action'];
if ($action == "update") {
 if (isset($_POST['delete'])) {
   $deleteList = join(', ', $_POST['delete']);
 }
 //Enter info into the database
 mysql_query("begin");
 foreach ($_POST['fleet_id'] as $key => $value) {
   $fleetCode = $_POST['fleet_code'][$key];
   $historyUrl = $_POST['history_url'][$key];
   $downloadUrl = $_POST['download_url'][$key];
   mysql_query("UPDATE imp_fleet SET fleet_code = '$fleetCode', history_url = 
'$historyUrl', download_url = '$downloadUrl' WHERE fleet_id = $value") or die 
(mysql_error());
 }
 if ($deleteList) {
   mysql_query("DELETE FROM imp_fleet WHERE fleet_id IN($deleteList)") or die 
(mysql_error());
 }
 if (mysql_error()) {
   echo ("There has been an error with your edit / delete request. Please contact the 
webmaster");
   mysql_query("rollback");
 } else {
   mysql_query("commit");
 }
}
?> <html> <head> <title></title> </head> <body> <form name="edit" method="post"> <h1>Edit / Delete Fleet</h1> <table> <tr> <td>Fleet Code</td> <td>Download URL</td> <td>History URL</td> <td>Delete</td> </tr> <? $sql = mysql_query("SELECT fleet_id, fleet_code, download_url, history_url FROM imp_fleet");

if (mysql_num_rows($sql) > 0) { while ($row = mysql_fetch_array($sql)) { ?> <tr> <td><input type="text" name="fleet_code[]" value="<?=$row['fleet_code']?>"><input type="hidden" name="fleet_id[]" value="<?=$row['fleet_id']?>"></td> <td><input type="text" name="download_url[]" value="<?=$row['download_url']?>"></td> <td><input type="text" name="history_url[]" value="<?=$row['history_url']?>"></td> <td><input type="checkbox" name="delete[]" value="<?=$row['fleet_id']?>"></td> </tr> <? } } ?> <tr> <td colsapn="4"> <table> <tr> <td><input type="hidden" name="action" value="update"><input type="reset" value="cancel"></td> <td colspan="2"><input type="submit" value="submit"></td> </tr> </table> </td> </tr> </table> </form> </body> </html>

Thanks for your time and feedback.

Matt


--
PHP General Mailing List (http://www.php.net/)
To unsubscribe, visit: http://www.php.net/unsub.php

Re: [PHP] Code Review PLEASE !!!

Reply via email to