Re: Code Review PLEASE !!!

If it were me I would do both Client and Server side validation. The majority of the time the client side will suffice, but, simply put, because you don't/may not look at the HTML source of a web page, doesn't mean that nobody else does. The fact of the matter is, you should not trust any data that comes from a form. Even if the ids come from the database, you still want to ensure that they really are a valid numerical value or whatever your ids happen to be based upon. Jordan S. Jones Matthew Oatham wrote:

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" <list@racistnames.com> To: "Matthew Oatham" <matthewoatham@hotmail.com>; <php-general@lists.php.net> 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