Bug in Image Upload to dir and database insert

[learning_brain]

Haven't been on for a while but I'm stuck!!!!!

I have a table with 3 image links in MySQL

In the add record page, I want to 1) upload the images to the server and 2) insert the paths to the database.

I'm referencing a tut at http://php.about.com/od/phpwithmysql...file_sql_3.htm

The form (or the main bits of it)

    <input name="title" type="text" id="title" size="50" />

    <label>Upload Image 1
    <input type="file" name="image_1" id="image_1" />
    </label>

    <label>Upload Image 2
    <input type="file" name="image_2" id="image_2" />
    </label>

    <label>Upload Image 3
    <input type="file" name="image_3" id="image_3" />
    </label>

The insert

//specify targets
    $target = "itemimages/"; 
    $target1 = $target . basename( $_FILES['image_1']['title']);
    $target2 = $target . basename( $_FILES['image_2']['title']);
    $target3 = $target . basename( $_FILES['image_3']['title']);

if ((isset($_POST["MM_insert"])) && ($_POST["MM_insert"] == "form1")) {
  $insertSQL = sprintf("INSERT INTO items (image_1, image_2, image_3, time_added, title, `description`, category, price) VALUES (%s, %s, %s, %s, %s, %s, %s, %s)",
                       $_FILES['image_1']['title'],
                       $_FILES['image_2']['title'],
                       $_FILES['image_3']['title'],
                       GetSQLValueString($_POST['time_added'], "int"),
                       GetSQLValueString($_POST['title'], "text"),
                       GetSQLValueString($_POST['description'], "text"),
                       GetSQLValueString($_POST['category'], "text"),
                       GetSQLValueString($_POST['price'], "double"));

  mysql_select_db($database_skinnerandhyde, $skinnerandhyde);
  $Result1 = mysql_query($insertSQL, $skinnerandhyde) or die(mysql_error()); 

The upload..

//Write image to server 
     if(move_uploaded_file($_FILES['image_1']['tmp_name'], $target1)) 
     {
         echo "The file ". basename( $_FILES['uploadedfile']['name']). " has been uploaded, and your information has been added to the directory"; 
     } 
     else {
         echo "Sorry, there was a problem uploading your file."; 
     } 
     

     if(move_uploaded_file($_FILES['image_2']['tmp_name'], $target2)) 
     {
         echo "The file ". basename( $_FILES['uploadedfile']['name']). " has been uploaded, and your information has been added to the directory"; 
     } 
     else {
         echo "Sorry, there was a problem uploading your file."; 
     }
     
 
     if(move_uploaded_file($_FILES['image_3']['tmp_name'], $target3)) 
     {
         echo "The file ". basename( $_FILES['uploadedfile']['name']). " has been uploaded, and your information has been added to the directory"; 
     } 
     else { 
         echo "Sorry, there was a problem uploading your file."; 
     } 

But I'm getting this error...

You have an error in your SQL syntax; check the manual that corresponds  to your MySQL server version for the right syntax to use near ' , ,  1307183343, 'test', 'sfgdfgdfhdghd', 'test', '9.99')' at line 1

So I tried with the sanitising function in the MySQL insert statement...

GetSQLValueString($_FILES['image_1']['title']),
GetSQLValueString($_FILES['image_2']['title']),
GetSQLValueString($_FILES['image_3']['title']),

and got this..

Warning:  Missing argument 2 for GetSQLValueString(), called in /home/skinne29/public_html/admin/add.php on line 89 and defined in /home/skinne29/public_html/admin/add.php on line 48

Warning:  Missing argument 2 for GetSQLValueString(), called in /home/skinne29/public_html/admin/add.php on line 90 and defined in /home/skinne29/public_html/admin/add.php on line 48

Warning:  Missing argument 2 for GetSQLValueString(), called in /home/skinne29/public_html/admin/add.php on line 91 and defined in /home/skinne29/public_html/admin/add.php on line 48
You have an error in your SQL syntax; check the manual that corresponds  to your MySQL server version for the right syntax to use near ' , ,  1307183690, 'gdfgdfg', 'dfgdgh', 'dfdhgdfh', '9.99')' at line 1

Could someone help to let me know where I'm going wrong?

Many thanks

Rich

[misson]

The MySQL error message tells you where the problem is. Notice the commas with no intervening values. There is no 'title' key in the elements of $_FILES. There is a 'name'. Since it comes from user input, you most assuredly need to worry about injection. You also need to consider what to do when a target filename already exists, otherwise the move_uploaded_file will overwrite the existing file.

To prevent injection, you shouldn't be escaping the values, you should be using prepared statements, which means you need to ditch the outdated mysql extension in favor of PDO.

It's a moot point, but the second error message also tells you the problem: the calls to GetSQLValueString are missing the second argument.

There's too much repetition in the PHP code. If you find yourself with variables that differ only in suffix, you should be using an array. If you're repeating code, you should be using a loop or a function.

[learning_brain]

Thanks Misson - I knew I could depend on you.

OK - good start! but I need to cover one thing at a time... Good point about filename - I'll concatenate some info to distinguish it - like the id.

Now my issue seems to be much more fundamental even with one upload....

//specify targets and define names
    $target = "http://www.skinnerandhyde.co.uk/item_images/"; 
    $target1 = $target.basename($_FILES['image_1']['name']);
    $image_name1 = $_FILES['image_1']['name']; 

Insert (works fine)

if ((isset($_POST["MM_insert"])) && ($_POST["MM_insert"] == "form1")) {
  $insertSQL = sprintf("INSERT INTO items (image_1, time_added, title, `description`, category, price) VALUES (%s, %s, %s, %s, %s, %s)",
                       GetSQLValueString($image_name1,"text"),      
               GetSQLValueString($_POST['time_added'], "int"),
                       GetSQLValueString($_POST['title'], "text"),
                       GetSQLValueString($_POST['description'], "text"),
                       GetSQLValueString($_POST['category'], "text"),
                       GetSQLValueString($_POST['price'], "double"));

  mysql_select_db($database_skinnerandhyde, $skinnerandhyde);
  $Result1 = mysql_query($insertSQL, $skinnerandhyde) or die(mysql_error()); 

The problem is here I think...

//Write image to server 
     if(move_uploaded_file($_FILES['image_1']['tmp_name'], $target1)) 
     {
         echo "The file ". basename( $_FILES['uploadedfile']['name']). " has been uploaded, and your information has been added to the directory"; 
     } 
     else {
         echo "Sorry, there was a problem uploading your file."; 
     } 

With the result...

Warning: move_uploaded_file() [function.move-uploaded-file]: Unable to move '/tmp/phpkt4A8W' to 'http://www.skinnerandhyde.co.uk/item_images/1.JPG' in /home/skinne29/public_html/admin/add.php on line 105
Sorry, there was a problem uploading your file.

Arrrgghhhhhh!

I also added a $_FILES['userfile']['error'], which returns a 0.... so the file is uploading but not moving.

Well at least the database is working fine. but the simple file upload isn't. What gives?

Rich

---------- Post added at 06:57 PM ---------- Previous post was at 04:52 PM ----------

OK - worked it out.

Apparently, it will not accept an absolute path for the destination.....???

So I just added ../item_images/ and it worked a treat.

Working on the other stuff now as well.

Thanks for looking.

Rich

[misson]

The time to start using PDO is now, before you start writing more code. You can go back and update existing code later.

//specify targets and define names
    $target = "http://www.skinnerandhyde.co.uk/item_images/"; 

Note this is a URL, not an absolute path.

//Write image to server 
     if(move_uploaded_file($_FILES['image_1']['tmp_name'], $target1)) 
     ... 

With the result...

Warning: move_uploaded_file() [function.move-uploaded-file]: Unable to move '/tmp/phpkt4A8W' to 'http://www.skinnerandhyde.co.uk/item_images/1.JPG' in /home/skinne29/public_html/admin/add.php on line 105
Sorry, there was a problem uploading your file.

[...]
Apparently, it will not accept an absolute path for the destination.....???

It will. It's URLs that won't work. Most URLs are read-only, and the ones that aren't require authentication first, so any function that can change what's stored at a given path generally doesn't support wrappers.

Try $_SERVER['DOCUMENT_ROOT'] . '/item_images' as the target base.

[jaggi190898]

Ageed with misson

[learning_brain]

Thanks Misson.

That worked a treat. I was getting some wrapper errors during testing so that explains a lot.

Still trying to get my head round PDO's.... :S

Rich

[misson]

Still trying to get my head round PDO's.... :S

Did you read the tutorial I recommended before? There's not much to using PDO. You don't even have to understand OOP, you just have to use OOP syntax: new to create a PDO object rather than mysql_connect, -> to call methods. At lower and the highest levels (interpreter implementation and theoretic, respectively), method calls are equivalent to function calls where the object is passed as a hidden parameter (named $this in PHP).

As for prepared statements, they're abstractions just like functions. Rather than repeating a piece of code with slight variations in the values, you can define a function that takes parameters. A prepared statement similarly lets you define a query once using PDO::prepare, with portions of it parameterized. PDOStatement::execute is analogous to function invocation.