Unlink and replace image

Hi

I want users to be able to replace a previous image they have uploaded with a new one, so I have a script that should unlink it and then replace it with a new file but with the same name. The original upload works so I think it is an issue with unlinking and replacing with the new image. I have posted the code below and if someone could have a look I would be very grateful.

<?php
$link = mysql_connect('','',''); 
if (!$link) { 
    die('Could not connect: ' . mysql_error()); 
} else { echo 'connected'; }
mysql_select_db(); 


$pagename = $_POST['pagename'];
$pagename = mysql_real_escape_string($pagename);


echo $pagename;


$query = ("SELECT * FROM bands WHERE PageName='$pagename'");


$result = mysql_query($query);


$row = mysql_fetch_array($result);


$bandid = $row['id'];


$allowedExts = array("jpg", "jpeg", "gif", "png");
$extension = end(explode(".", $_FILES["ufile"]["name"]));
if ((($_FILES["ufile"]["type"] == "image/gif")
|| ($_FILES["ufile"]["type"] == "image/jpeg")
|| ($_FILES["ufile"]["type"] == "image/pjpeg"))
&& ($_FILES["ufile"]["size"] < 20000)
&& in_array($extension, $allowedExts))
  {
  if ($_FILES["file"]["error"] > 0)
    {
    echo "Return Code: " . $_FILES["ufile"]["error"] . "<br />";
    }
else {
$file_name = $HTTP_POST_FILES['ufile']['name'];


$random_digit= $bandid ;


$new_file_name=$random_digit.'.'.$extension;


if (file_exists("upload/$new_file_name")) 
    unlink("upload/$new_file_name");


$path= "upload/".$new_file_name;
if($ufile !=none)
{
if(move_uploaded_file($HTTP_POST_FILES['ufile']['tmp_name'], $path))
{
echo "Successful<BR/>"; 


echo "File Name :".$new_file_name."<BR/>"; 


echo "File Size :".$HTTP_POST_FILES['ufile']['size']."<BR/>"; 


echo "File Type :".$HTTP_POST_FILES['ufile']['type']."<BR/>"; 


echo $bandid;


$buery = ("UPDATE bands SET imageurl = '$path' WHERE id='$bandid'");


$besult = mysql_query($buery); 
}
else
{
echo "Error";
}
}
}
  }
}
?>
   

Code (markup):

The connection to Mysql works, and $pagename echos correctly, but none of the messages after move_uploaded_file echo.

Any help is very appreciated
Gilgil

 

I'm not seeing anything wrong with the unlink, but you have a variable, $ufile, which doesn't seem to actually be defined anywhere... and ==none isn't exactly a valid construct... there are also a few unused variables, and the inconsistent swapping in and out of single and double quotes, lack of code indentation, and in a number of places out-right brute forcing of things PHP already has variables to handle... and the use of mysql_ functions which aren't really secure and require all sorts of extra escaping...

Let's just say some improvements could be made.

I THINK (this is untested code) this would work a bit better.

<?php

function errorHandler($message) {
	die('
		<h2>The Following Error Was Encountered:</h2>
		<p>'.$message.'</p>
	');
}

// process the file errors FIRST, no point in continuing if it went awry
if ($_FILES['file']['error']>0) errorHandler('
	File Error: '.$_FILES['ufile']['error']
);

try {
	$db=new PDO(
		'mysql:dbname=test;host=localhost',
		'dbuser',
		'dbpass'
	);
	echo 'Connected<br />';
} catch (PDOException $e) errorHandler('
	Unable to Connect To Database
');

echo 'attempting: ',htmlspecialchars($pagename),'<br />';

$statement=$db->prepare("
	SELECT * FROM bands
	WHERE PageName = :pageName
");
$statement->execute(array(':pageName' => $pagename);

if ($band=$statement->fetch()) {

	$allowedMimeTypes=array('image/gif','image/jpeg','image/pjpeg','image/png');
	$allowedExts=array('jpg', 'jpeg', 'gif', 'png');
	$extension=pathinfo($_FILES['ufile']['name'],PATHINFO_EXTENSION);
	
	if (!in_array($_FILES['ufile']['type'],$allowedMimeTypes)) errorHandler('
		Incorrect Mime-type '.$_FILES['ufile']['type'].'<br />
		Must be one of '.implode(', ',$allowedMimeTypes)
	);
	if (!in_array($extension,$allowedExts)) errorHandler('
		Disallowed Extension '.$extension.'<br />
		Must be one of '.implode(', ',$allowedExts)
	);
	if ($files['ufile']['size']>20480) errorHandler('
		File too large, Must be less than 20k
	');
	$newFileName='upload/'.$band['id'].'.'.$extension;

	if (file_exists($newFileName)) unlink($newFileName);

	if (move_uploaded_file(
		$HTTP_POST_FILES['ufile']['tmp_name'],
		$path
	)) {
		echo '
			Successful<br/>
			File Name :',$new_file_name,'<br />
			File Size :',$HTTP_POST_FILES['ufile']['size'],'<br />
			File Type :',$HTTP_POST_FILES['ufile']['type'],'<br />
			',$band['id'];
			
		$statement=$db->prepare("
			UPDATE bands
			SET imageurl = :imagePath
			WHERE id = :bandId
		");
		$statement->execute(array(
			':imagePath' => $newFileName,
			':bandId' => $band['id']
		));
	} else errorHandler('Unable to move uploaded file');

} else errorHandler('Band Not Found / Invalid Band ID');

?>

Code (markup):

No guarantees though on how well that would work. Should be functionally identical just a bit cleaner and modern a codebase... naturally swap out _database, _username and _password, (and localhost if need be) for your values.

 

Thanks deathshadow, I did solve it but forgot to say so on here, the problem was that the error messages weren't showing correctly so I changed it round and it works now. However yours does look much better and uses PDO etc. so will make a not of this and swap it in when I have time.

Thanks

 

In that case you should look at the lines BEFORE move_uploaded_file :
what does the

if ($ufile != none)

Code (markup):

mean? Have you declared somewhere at the beginning of your code $ufile = "none"; ? Without quotes I think PHP doesn't 'know' what none is...
Also, I can't see where does the if statement

if (file_exists("upload/$new_file_name")) 
    unlink("upload/$new_file_name");

Code (markup):

ends? There's no endif,/ nor opening/closing {}
Try to clean up your code, don't use error messages "ERROR", but refer somehow to the error that has occured (i.e. "ERROR with validating $ufile value...").

EDITED : obviously I've submitted my reply a bit late....