Getting file extension

[herghost]

Evening everyone

 

I am having some problems trying to get the file extension from an image upload script. This is what I have:

 

<?php
$upload_dir = 'upload/'; // Directory for file storing
$preview_url = 'upload/';
$p_id = $_COOKIE["fn"];
$filename= '';
$ext = substr(strrchr($filename, '.'), 1);
$result = 'ERROR';
$result_msg = '';
$allowed_image = array ('image/png','image/gif', 'image/jpeg', 'image/jpg', 'image/pjpeg');
define('PICTURE_SIZE_ALLOWED', 2242880); // bytes

if (isset($_FILES['picture']))  // file was send from browser
{
if ($_FILES['picture']['error'] == UPLOAD_ERR_OK)  // no error
{
if (in_array($_FILES['picture']['type'], $allowed_image)) {
if(filesize($_FILES['picture']['tmp_name']) <= PICTURE_SIZE_ALLOWED) // bytes
{

$prefile = md5($p_id);
$filename= $prefile .'.'. $ext;
move_uploaded_file($_FILES['picture']['tmp_name'], $upload_dir.$filename);

 

Well thats the part I am concerned with anyway!

 

Basically the file uploads fine, but it is just called the result of the md5 hash and does not have the extension. Any idea on where I am going wrong?

 

The full script is below:

 

index.php

<script>
function ajaxFileUpload(upload_field)
{
// Checking file type
var re_text = /\.png|\.jpg|\.gif|\.jpeg/i;
var filename = upload_field.value;
if (filename.search(re_text) == -1) {
alert("File should be either jpg, jpeg, gif or png");
upload_field.form.reset();
return false;
}
document.getElementById('picture_preview').innerHTML = '<div><img src="images/progressbar.gif" border="0" /></div>';
upload_field.form.action = 'upload-picture.php';
upload_field.form.target = 'upload_iframe';
upload_field.form.submit();
upload_field.form.action = '';
upload_field.form.target = '';
return true;
}
</script>

<!-- iframe used for ajax file upload-->
<!-- debug: change it to style="display:block" -->
<link href="Styles/form_clean.css" rel="stylesheet" type="text/css" />
<iframe name="upload_iframe" id="upload_iframe" style="display:none;"></iframe>
<!-- iframe used for ajax file upload-->
<form name="pictureForm" method="post" autocomplete="off" enctype="multipart/form-data" class="clean">
  <ol>
    <li>
      <fieldset>
        <legend>Step 3 - Image</legend>
        <ol>
        <input type="file" name="picture" id="picture" onchange="return ajaxFileUpload(this);" />
<span id="picture_error"></span>
<div id="picture_preview"></div>
</div>
        </ol>
      </fieldset>
    </li>
  </ol>
  <p style="text-align:right;">
    <input type="reset" value="CANCEL" />
    <input type="submit" value="OK" />
  </p>
</form>

 

upload-picture.php

 

<?php
$upload_dir = 'upload/'; // Directory for file storing
$preview_url = 'upload/';
$p_id = $_COOKIE["fn"];
$filename= '';
$ext = substr(strrchr($filename, '.'), 1);
$result = 'ERROR';
$result_msg = '';
$allowed_image = array ('image/png','image/gif', 'image/jpeg', 'image/jpg', 'image/pjpeg');
define('PICTURE_SIZE_ALLOWED', 2242880); // bytes

if (isset($_FILES['picture']))  // file was send from browser
{
if ($_FILES['picture']['error'] == UPLOAD_ERR_OK)  // no error
{
if (in_array($_FILES['picture']['type'], $allowed_image)) {
if(filesize($_FILES['picture']['tmp_name']) <= PICTURE_SIZE_ALLOWED) // bytes
{

$prefile = md5($p_id);
$filename= $prefile .'.'. $ext;
move_uploaded_file($_FILES['picture']['tmp_name'], $upload_dir.$filename);

//phpclamav clamscan for scanning viruses
//passthru('clamscan -d /var/lib/clamav --no-summary '.$upload_dir.$filename, $virus_msg); //scan virus
$virus_msg = 'OK'; //assume clamav returing OK.
if ($virus_msg != 'OK') {
unlink($upload_dir.$filename);
$result_msg = $filename." : ".FILE_VIRUS_AFFECTED;
$result_msg = '<font color=red>'.$result_msg.'</font>';
$filename = '';
}else {
// main action -- move uploaded file to $upload_dir
$result = 'OK';
}
}else {
$filesize = filesize($_FILES['picture']['tmp_name']);// or $_FILES['picture']['size']
$filetype = $_FILES['picture']['type'];
$result_msg = PICTURE_SIZE;
}
}else {
$result_msg = SELECT_IMAGE;
}
}
elseif ($_FILES['picture']['error'] == UPLOAD_ERR_INI_SIZE)
$result_msg = 'The uploaded file exceeds the upload_max_filesize directive in php.ini';
else
$result_msg = 'Unknown error';
}

// This is a PHP code outputing Javascript code.
echo '<script language="JavaScript" type="text/javascript">'."\n";
echo 'var parDoc = window.parent.document;';
if ($result == 'OK') {
echo 'parDoc.getElementById("picture_error").innerHTML =  "";';
}
else {
echo "parDoc.getElementById('picture_error').innerHTML = '".$result_msg."';";
}

if($filename != '') {
echo "parDoc.getElementById('picture_preview').innerHTML = '<img src=\'$preview_url$filename\' id=\'preview_picture_tag\' heigh=\'100\' width=\'100\' name=\'preview_picture_tag\' />';";
}

echo "\n".'</script>';
exit(); // do not go futher

?>

[AyKay47]

you are setting the $filename variable to an empty string, then trying to use substr() to find the extension of the empty string.

How do you expect this to work? You will want to utilize something like this instead..

 

<?php
$upload_dir = 'upload/'; // Directory for file storing
$preview_url = 'upload/';
$p_id = $_COOKIE["fn"];
$result = 'ERROR';
$result_msg = '';
$allowed_image = array ('image/png','image/gif', 'image/jpeg', 'image/jpg', 'image/pjpeg');
define('PICTURE_SIZE_ALLOWED', 2242880); // bytes

if (isset($_FILES['picture']))  // file was send from browser
{
if ($_FILES['picture']['error'] == UPLOAD_ERR_OK)  // no error
{
if (in_array($_FILES['picture']['type'], $allowed_image)) {
if(filesize($_FILES['picture']['tmp_name']) <= PICTURE_SIZE_ALLOWED) // bytes
{

$prefile = md5($p_id);
$file_name = $_FILES['picture']['name'];
$explode = explode(".",$file_name);
$ext = $explode[1];
$filename= $prefile"."$ext;
move_uploaded_file($_FILES['picture']['tmp_name'], $upload_dir.$filename);

[.josh]

file names can have more than 1 dot in it, you shouldn't rely on the extension being the 2nd element of the exploded array...you should instead check for the last element in the array.

 

$extension = array_pop(explode(".",$file_name));

 

Though that's not guaranteed to be the extension (or not).  If you have for instance "file" then it will return "file".  Better method:

 

$extension = pathinfo($file_name,PATHINFO_EXTENSION);

 

[AyKay47]

file names can have more than 1 dot in it, you shouldn't rely on the extension being the 2nd element of the exploded array...you should instead check for the last element in the array.

 

$extension = array_pop(explode(".",$file_name));

 

Though that's not guaranteed to be the extension (or not).  If you have for instance "file" then it will return "file".  Better method:

 

$extension = pathinfo($file_name,PATHINFO_EXTENSION);

good point, didn't think to use pathinfo(), which would be a much better, cleaner solution here, thanks for that