uploader not working

[wincode]

Hi, I had this picture uploading script that worked perfectly when I tested it 2-3 weeks ago. I tried it again today, and I'm getting the error "No file selected."

<?php
if($_GET['add']==1){
$name= $_FILES['avatarfile']['name'];
$type= $_FILES['avatarfile']['type'];
$size= $_FILES['avatarfile']['size'];
$tmpname= $_FILES['avatarfile']['tmp_name'];
$error= $_FILES['avatarfile']['error'];

if($name)
{

//conditions for the file
$allowed=array("image/jpeg", "image/jpg", "image/png", "image/gif");
if (in_array($type, $allowed))
{
$randomdig=rand(0000,9999);
$namenew=$randomdig.$name;
$picturelocation="avatar/".$namenew;
//include('thumbs.php');
move_uploaded_file($tmpname, $picturelocation);
}else{
echo"File extension not allowed.";
}
}else{
echo"No file selected.";
}
}
else{
echo'<form action="upload.php?add=1" method="post">
<input type="file" name="avatarfile">
<input type="submit" value="Add" name="submit">
</form>';
}

?>

What could have went wrong/why isn't it working? I didn't change anything on the page, but some how it stopped working. I've checked the chmod on the folders too, all fine.

Thanks for your help

[wincode]

I figured it out, I forgot to add enctype="multipart/form-data" :p
Sorry for the posts

[Matthew1980]

Hi there Wincode,

>>if($_GET['add']==1){

All your asking there is if it's equal to 1, when realistically you need to add more security checks first so that you protect your site a little better.

Try:-

if(isset($_GET['add']) && !empty($_GET['add']) && ($_GET['add'] == 1)){
//all set not empty & equal to what your expecting!
}
else{
//redirect back to form as form not submitted correctly
}

Hopefully you see the idea there, you can be as strict as you want, I just showed you some options there.

Cheers,
MRb

[Readie]

if(isset($_GET['add']) && !empty($_GET['add']) && ($_GET['add'] == 1)){ 

Even better, check for the existence of the $_FILES variable:

if(isset($_FILES['avatarfile']) && is_array($_FILES['avatarfile']) && !empty($_FILES['avatarfile'])) {

[Matthew1980]

^^^

Hi there Readie, not being too pedantic, but, that if clause your suggesting would be the next line in - Check form submitted the correct way (not by command line etc), then check that the $_FILES array has been set and has value - if those conditions are true, then you can carry or processing the data..

You could condense it all into a smaller single clause, but splitting it up does seem more diligent IMO.

if((isset($_GET['add']) && !empty($_GET['add'])) && (isset($_FILES['avatarfile']) && !empty(['avatarfile']))){
//as you were
}
else{
//failed
}

Well, it's an option I guess...

Cheers,
MRb

Cheers,
MRb

[Readie]

I was suggesting it as a replacement to the one you offered, but I think the solution you offered in the post directly above this would be neater. You're checking you're at the right URL and the form has been posted correctly.

I also think that somewhere further down, we need an

if($_FILES['avatarfile']['error']) {
    // Echo out some error message
} else {
    // Continue processing
}

[rocknbil]

The first thing I would look at is this:

$picturelocation="avatar/".$namenew;

This should be a full path. NOT A URL.

$picturelocation="avatar/".$namenew;

$full_path = $_SERVER['DOCUMENT_ROOT'] . "/$picturelocation";

I don't know that this will help, but there's no need to mix get and post. Just use a hidden. You're uploading a file, right? Just check for elements required to do the upload - $_FILES.

<?php  
$errors=null;  
// HERE you can check for an empty "avatarfile."  
if (isset($_POST['add'] and empty($_FILES['avatarfile'])) {  
 $errors .= "<li>You didn't select a file to upload.</li>";  
}  
if($_FILES){  
 $name= $_FILES['avatarfile']['name'];  
 $type= $_FILES['avatarfile']['type'];  
 $size= $_FILES['avatarfile']['size'];  
 $tmpname= $_FILES['avatarfile']['tmp_name'];  
 $error= $_FILES['avatarfile']['error'];  
 if ($error) { $errors .= "<li>File upload error: $error</li>"; }  
 else {  
 // Checking $name is irrelevant, you want to check the file, right?  
 //conditions for the file  
 $allowed=array("image/jpeg", "image/jpg", "image/png", "image/gif");  
 if (in_array($type, $allowed)) {  
  $randomdig=rand(0000,9999);  
  $namenew=$randomdig.$name;  
  $picturelocation="avatar/".$namenew;  
  $full_path = $_SERVER['DOCUMENT_ROOT'] . "/$picturelocation";  
  //include('thumbs.php');  
  move_uploaded_file($tmpname, $full_path);  
  // You need a response here  
  echo "</p>The file $name was uploaded and saved as <a href=\"/$picturelocation\">$newname</a></p>";  
 }  
 else{ $errors .= "<li>File type not allowed.</li>"; }  
} // End if $_FILES 
if ($errors) {  
 echo "<p>The following errors have occurred: <ul> $errors </ul>";  
}  
echo '<form action="upload.php method="post" enctype="multipart/form-data">  
<input type="hidden" name="add" value="1">  
<input type="file" name="avatarfile">  
<input type="submit" value="Add" name="submit">  
</form>';  
?>  

May contain syntax errors . . . go forth and debug.