Editing issue

[zimmo]

How can I modify my script below for editing correctly. It uses an if else statement at the moment to check that the image is actually being sent from the form, and if it is it will re-upload it and update the name and other data in the database, if it does not it will use the other sql statement and update only the text, as I miss out the image field.

Now, is there a solution here so that I can have one piece of code.

This would check to see if the user is uploading an image from the form, so a isset check on my upload side of the script.

Then to update the sql table if they have uploaded a new image?

As I can do the image check, but then if they decide not to upload an image, the data name in the database would re-enter but it would enter nothing, leaving me with a blank field in the table and not the old image name.

<?php


// No image
 
 if ($_SERVER['REQUEST_METHOD'] =='POST' && empty($_FILES['image']['name'])) {

 // clean out any malicious data
 foreach ($_REQUEST as $k => $v) {
 $_REQUEST[$k] = (get_magic_quotes_gpc() ? strip_tags($v) : strip_tags(addslashes($v)));
 }

  # setup SQL statement for no new image
  $SQL = " UPDATE music_books SET category = '{$_POST['category']}', author_composer = '{$_POST['author_composer']}', a_z = '{$_POST['a_z']}', title = '{$_POST['title']}', description = '{$_POST['description']}', price = '{$_POST['price']}' WHERE id = '{$_REQUEST['id']}' ";
}

// New Image

 elseif ($_SERVER['REQUEST_METHOD'] =="POST") {

// clean out any malicious data
 foreach ($_REQUEST as $k => $v) {
 $_REQUEST[$k] = (get_magic_quotes_gpc() ? strip_tags($v) : strip_tags(addslashes($v)));
 }

// Check the image type is a jpeg or gif for the image.
 if (($_FILES['image']['type'] != "image/gif") && ($_FILES['image']['type'] != "image/jpeg") && ($_FILES['image']['type'] != "image/pjpeg")) {
  echo "<FONT FACE=\"Verdana\"><SPAN CLASS=\"style100\">You have chosen not up upload a image at this time.<BR></SPAN>" ;
 } elseif ($_FILES['image']['size'] > 300000) {
  echo "<FONT FACE=\"Verdana\"><SPAN CLASS=\"style100\">The file size is bigger than 300kb.<BR></SPAN>" ;
 } else {
  move_uploaded_file($_FILES['image']['tmp_name'], "/music_book_images/".$_FILES['image']['name']) ;
 echo "<FONT FACE=\"Verdana\"><SPAN CLASS=\"style100\"><B>Your image has successfully uploaded.</B><BR></SPAN>" ;
  }

   # setup SQL statement for new logo
  $SQL = " UPDATE music_books SET category = '{$_POST['category']}', author_composer = '{$_POST['author_composer']}', a_z = '{$_POST['a_z']}', title = '{$_POST['title']}', description = '{$_POST['description']}', image = '{$_FILES['image']['name']}', price = '{$_POST['price']}' WHERE id = '{$_REQUEST['id']}' ";
}
  
  #execute SQL statement
  $result = mysql_db_query( ****,"$SQL",$connection );

  # check for error
  if (!$result) { echo("ERROR: " . mysql_error() . "\n$SQL\n");    }
?>

Hope someone can point me in the right direction.

[bokeh]

<?php

if ($_SERVER['REQUEST_METHOD'] =="POST")
{
    // this is crap... also should be using mysql_real_escape_string not addslashes
    foreach ($_REQUEST as $k => $v) 
    {
        $_REQUEST[$k] = (get_magic_quotes_gpc() ? strip_tags($v) : strip_tags(addslashes($v)));
    }
    
    // Check the image type is a jpeg or gif for the image.
    if((empty($_FILES['image']['tmp_name'])) or !(@getimagesize($_FILES['image']['tmp_name'])))
    {
        // try to avoid presentational mark-up
        echo "<FONT FACE=\"Verdana\"><SPAN CLASS=\"style100\">You have chosen not up upload a image at this time.<BR></SPAN>" ;
    }
    elseif($_FILES['image']['size'] > 300000)
    {
        // try to avoid presentational mark-up
        echo "<FONT FACE=\"Verdana\"><SPAN CLASS=\"style100\">The file size is bigger than 300kb.<BR></SPAN>" ;
    } 
    else 
    {
        $success = move_uploaded_file($_FILES['image']['tmp_name'], "/music_book_images/".$_FILES['image']['name']) ;
        // try to avoid presentational mark-up
        echo "<FONT FACE=\"Verdana\"><SPAN CLASS=\"style100\"><B>Your image has successfully uploaded.</B><BR></SPAN>" ;
    }
    
    $image = $success ? "image = '{$_FILES['image']['name']}'," : null ;
    
    # setup SQL statement for new logo
    $SQL = "UPDATE music_books SET category = '{$_POST['category']}', author_composer = '{$_POST['author_composer']}', a_z = '{$_POST['a_z']}', title = '{$_POST['title']}', description = '{$_POST['description']}', $image price = '{$_POST['price']}' WHERE id = '{$_REQUEST['id']}' ";

    #execute SQL statement or echo error
    $result = mysql_db_query( '****', "$SQL",$connection ) or print("ERROR: " . mysql_error() . "\n$SQL\n");
}

?>

[zimmo]

Hi Bokeh,

I have read some of your other posts on this forum before and especially the ones where you have been having a long discussion and ending up proving people wrong. So thanks for the reply.

I am always learning new ways of writing php.

So, could you explain to me how you would change things here and what I should be using instead:

 // this is crap... also should be using mysql_real_escape_string not addslashes 
 foreach ($_REQUEST as $k => $v) 
 { 
 $_REQUEST[$k] = (get_magic_quotes_gpc() ? strip_tags($v) : strip_tags(addslashes($v))); 
 }

Also, why is it not wise to use presentation mark up?
Should I be using style sheets and not doing things this way?

The only reason i do this is due to the old software I still use for cms systems I put together.

[bokeh]

could you explain to me how you would change things here and what I should be using instead:

 // this is crap... also should be using mysql_real_escape_string not addslashes 
 foreach ($_REQUEST as $k => $v) 
 { 
 $_REQUEST[$k] = (get_magic_quotes_gpc() ? strip_tags($v) : strip_tags(addslashes($v))); 
 }

Well there a few things: In the query why are you using $_REQUEST and $_POST. Personally I don't like using $_REQUEST at all (not that it is wrong) but it is impossible to tell where the variable came from and possible to have collisions. Also I don't like dynamic handling of user input nor modification of the superglobal arrays. I would do away with the above code altogether and use a function on each variable while building the query.

<?php

# I'd be testing a variable here instead... Allows for multiple step $_POSTs
if ($_SERVER['REQUEST_METHOD'] =="POST") 
{
    if((empty($_FILES['image']['tmp_name'])) or !(@getimagesize($_FILES['image']['tmp_name'])))
    {
        echo "<FONT FACE=\"Verdana\"><SPAN CLASS=\"style100\">You have chosen not up upload a image at this time.<BR></SPAN>" ;
    }
    elseif($_FILES['image']['size'] > 300000)
    {
        echo "<FONT FACE=\"Verdana\"><SPAN CLASS=\"style100\">The file size is bigger than 300kb.<BR></SPAN>" ;
    } 
    else 
    {
        $success = move_uploaded_file($_FILES['image']['tmp_name'], "/music_book_images/".$_FILES['image']['name']) ;
        echo "<FONT FACE=\"Verdana\"><SPAN CLASS=\"style100\"><B>Your image has successfully uploaded.</B><BR></SPAN>" ;
    }
    
    # must come before using mysql_real_escape_string
    @mysql_connect($host, $user, $pass) or die('Time to call an expert!');
    @mysql_select_db($database) or die(mysql_error());
        
    $image = $success ? "image = ".clean($_FILES['image']['name'])."," : null ;
    # setup SQL statement for new logo
    $query = "UPDATE music_books SET category = ".clean($_POST['category']).", author_composer = ".clean($_POST['author_composer']).", a_z = ".clean($_POST['a_z']).", title = ".clean($_POST['title']).", description = ".clean($_POST['description']).", $image price = ".clean($_POST['price'])." WHERE id = ".clean($_REQUEST['id']);

    #execute SQL statement or echo error
    $result = mysql_query($query) or print("ERROR: " . mysql_error() . "\n$SQL\n");
}

function clean($input)
{
    if(get_magic_quotes_gpc())
    {
        $input = stripslashes($input);
    }
    if(is_numeric($input))
    {
        return $input;
    }
    return "'".mysql_real_escape_string($input)."'";
}

?>

Also, why is it not wise to use presentation mark up?

The present take on things (according to W3C) is:

• Content: HTML
• Presentation: CSS
• Behaviour: Javascript