Yes I agree I need some validation, dunno whether to do server or client
side validation. I don't think the fleet_id example will be a problem though
as this is retrieved from the database where the field is an int.
Thanks for your feedback
Matt
----- Original Message -----
From: "Jordan S. Jones" <[EMAIL PROTECTED]>
To: "Matthew Oatham" <[EMAIL PROTECTED]>;
<[EMAIL PROTECTED]>
Sent: Monday, April 05, 2004 11:56 PM
Subject: Re: [PHP] Code Review PLEASE !!!
> 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
>
>
--
PHP General Mailing List (http://www.php.net/)
To unsubscribe, visit: http://www.php.net/unsub.php
