Not really, it’s moving the images to the same directory as the script, meaning the scripts directory would need write permissions – and that’s usually a no-no.
Though I’m scratching my head at the bizarre mix of string styles… the double quotes for slow, the stupid ‘nowdoc’ thing I’ll NEVER understand the advantage of… total lack of clear formatting in the code (or did the forum strip that out?), multiple echo’s doing the job of a single one.
In any case, I’d modify it to move the image to a subdirectory instead of the one the script is executing out of, that way you aren’t running scripts from a directory with write permissions.
Thank you deathshadow60, if I set the upload directory is less bad? Okay I will modify it. But show me where I can find a tutorial that gives me idea how to create a clean upload script?
I went through and ‘cleaned it up’ a little, adding the code for it to use a subdirectory. I also added a ‘file_exists’ check so that you can upload files with the same name as different files on the server – a [1], [2], [3] suffix is added to the base filename as needed, and I also do a urlencode on the filename to reduce issues linking to the files. A check is also added when moving the file so that if the target directory doesn’t have write permissions or you run out of disk space or what-have-you, it throws a meaningful error… I put the directory to move the file into in as a variable at the top for easy modification… and I cleaned up the markup into something a wee bit more valid and modern.
<?php
$imageDirectory='images/';
?>
<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Strict//EN"
"http://www.w3.org/TR/xhtml1/DTD/xhtml1-strict.dtd">
<html
xmlns="http://www.w3.org/1999/xhtml"
lang="en"
xml:lang="en"
><head>
<meta
http-equiv="Content-Type"
content="text/html; charset=utf-8"
/>
<meta
http-equiv="Content-Language"
content="en"
/>
<title>
PHP Form Upload
</title>
</head><body>
<form method="post" action="imageUpload.php" enctype="multipart/form-data">
<fieldset>
<label for="uploadFilename">
Select a JPEG, GIF, PNG or TIF File:
</label><br />
<input type="file" name="filename" id="uploadFileName" size="24" />
<input type="submit" value="Upload" />
</fieldset>
</form>
<?php
if (isset($_FILES['filename'])) {
$mimeLookup=array(
'image/jpeg' => '.jpg',
'image/gif' => '.gif',
'image/png' => '.png',
'image/tiff' => '.tif'
);
if (isset($mimeLookup[$_FILES['filename']['type']])) {
$ext=$mimeLookup[$_FILES['filename']['type']];
$name=urlencode( // make sure we have a 'safe' for web filename
pathInfo(
$_FILES['filename']['name'],
PATHINFO_FILENAME
)
);
$found=0;
do {
$localPath=$imageDirectory.$name.(
$found==0 ? '' : '['.$found.']'
).$ext;
$found++;
} while (file_exists($localPath));
if (move_uploaded_file($_FILES['filename']['tmp_name'],$localPath)) {
echo (
$found==1 ? '' : 'Existing file with same name already present'
),'
<h1>
Uploaded image <var>',$_FILES['filename']['name'],'</var>
as <kbd>',$localPath,'</kbd>
</h1>
<img src="',$localPath,'" alt="uploaded image" />';
} else {
echo '
<h1>
*ERROR*
Was unable to move <var>',$_FILES['filename']['name'],'</var>
</h1>
<p>
Are you sure the <var>',$imageDirectory,'</var> directory has write permissions?
</p>';
}
} else echo '
<h1>
*ERROR*
Invalid image format
</h1>
<p>
Files of type <var>',$_FILES['filename']['type'],'</var> are not accepted by this site.
</p>';
} else {
echo '
<p>
No image has been uploaded.
</p>';
}
?>
</body></html>
Hope this helps. The ‘exists’ code could be expanded further by a filesize and then a contents check, so you don’t end up with duplicates of the same file… though that would add more overhead to an already slow process.
(btw, this rewrite breaks my general rule of “only one <?php ?>” pairing per file, but was done to keep it easier for you to follow. I’d not do that in production code, where everything from body up and everything from </body> down would be in functions in a external file…)
Still not good, you are still looking using user input to identify the files. “$_FILES[‘filename’][‘type’]” is bad never use it. Instead there are a couple of things you need to do. first determine if it is an image. exif_imagetype or [URL=“http://www.php.net/manual/en/function.getimagesize.php”]getimagesize both can help with that. Next, pick a new name for the file, can keep the old one if you like but change the extension to match the image type. (JPEG should have a .jpg ext, PNG should be .png, etc.)
Assuming PHP is compiled with the exif included (it often isn’t), assuming windows users have mb_string enabled, assuming the file is 12 bytes or more long… Mind you, I like it and would say use exactly that, but it’s often not available.
Which thankfully while considered part of the GD module, does NOT require GD to function. This is probably the better choice for checking it.
Which the code is already set up to do, you just change where you’re pulling the mime-type from.
So… yeah, replace this:
if (isset($mimeLookup[$_FILES['filename']['type']])) {
$ext=$mimeLookup[$_FILES['filename']['type']];
With this:
$imageInfo=getImageSize($_FILES['filename']['tmp_name']);
if (isset($mimeLookup[$imageInfo['mime']])) {
$ext=$mimeLookup[$imageInfo['mime']];
One and the same… the PURPOSE of PHP in web development is to take user input (from the URL itself, the filename iteself, GETDATA or POSTDATA), make calls to the appropriate filesystem or database systems, then output the result as HTML… so any PHP worth using on a website typically outputs markup or contains large amounts of markup.
Everything in that code box should be saved as a single .php file.
Really it’s what PHP does best – taking the user request, accessing existing library functions or other parts of the system to make a result, and then outputting that result inside some html. It’s why PHP has the massive function library as, well… PHP is best used as glue… to glue the request to a query or processing, then glue the results of that processing to some markup… It’s also why trying to use PHP for ‘hard computing’ is usually less than effective.