PHP File Uploading Error

Question

I am developing a website using the Model-View-Controller structure. In my html, I have a form that allows the user to browse and select a file, and submit the file by clicking the button which then calls the controller code:

if(upload_quotes($_FILES['userfile'])){
    $msg = "Successful file upload";  
    $directory_list = array();

    // get contents of directory here
    // $directory_list = get_directory_contents($directory_list);

    include '../view/processFileUpload.php';
}else{
    $msg = "Unknown error occurred.  Error: ".print_r($_FILES);
    include '../view/errorPage.php';
}

The first line: upload_quotes() is a function located in the model code. The function returns true if successful upload and false if not successful. I have commented out the function call that gets the directory listing because there are errors in it as well. The 2 model code snippets:

function upload_quotes($_FILES){
    $uploadfile = '../../../TestSiteDataFiles/Quotes/'.$_FILES['userfile']['name'];
    if(move_uploaded_file($_FILES['tmp_name'], $uploadfile)){
        return true;
    }else{
        return false;
    }
}

function get_directory_contents($directory_list){
    $current_dir = '../../../TestSiteDataFiles/Quotes/';
    $dir = opendir($current_dir);

    //reads and outputs the directory
    while(false !== ($file = readdir($dir)))
    //strip out the two entries of . and ..
    if($file != "." && $file !=".."){
        array_push($directory_list, $file);
    }
    closedir($dir);
    return $directory_list;
}

The processFileUpload.php and errorPage.php files output the $msg variable accordingly however it never outputs as successful. I've spent hours researching what i've done wrong and my limited knowledge of php isn't any help. The view page outputs "Unknown error occured. Error: 1. And the errors that pop up on the browser are:

Warning: move_uploaded_file() [function.move-uploaded-file]: The second argument to copy() function cannot be a directory in C:\xampp\htdocs\Test Site\model\model.php on line 17

Warning: move_uploaded_file() [function.move-uploaded-file]: Unable to move 'C:\xampp\tmp\php487A.tmp' to '../../../TestSiteDataFiles/Images/' in C:\xampp\htdocs\Test Site\model\model.php on line 17 Array ( [name] => Managerial Accounting Davers Connect.txt [type] => text/plain [tmp_name] => C:\xampp\tmp\php487A.tmp [error] => 0 [size] => 55 )

It appears to me as the model code (upload_quotes()) is the source of the error since it returns as false everytime. the get_directory_contents() function never has a chance of executing but it does not output the correct results either.

I appreciate any and all suggestions and thank you for your inputs.

Answer 1

By passing $_FILES to your function as a parameter, you have confused the source of your file info in the actual $_FILES superglobal. Since you access $_FILES['userfile']['name'] in the function, but pass $_FILES['userfile'] to the function, the name key is not defined, and you get the directory error.

It is incredibly dangerous to use the user input filename to store a file on your filesystem. Instead, it is better to create a filename that will be unique. The original input filename from $_FILES[]['name'] is useful to store in a database and associate with a file on disk as metadata to display, but it shouldn't be used to store on disk.

// Use a different variable name. I've replaced it with $fileinfo
function upload_quotes($fileinfo){

    // Don't use the original filename to store it. Create one instead.
    $fname = uniqid();
    $finfo = pathinfo($fileinfo['name']);
    // Append the user's file extension to a random filename
    $fname .= "." . $finfo['extension'];

    $uploadfile = '../../../TestSiteDataFiles/Quotes/'.$fname;

    // Don't attempt to move the file unless its error container is empty
    if(empty($fileinfo['error']) && move_uploaded_file($fileinfo['tmp_name'], $uploadfile)){
        return true;
    }else{
        return false;
    }
}

Update

The get_directory_contents() function fails because the array $directory_list is defined out of scope. There's no need to define it as an array before calling the function. Do it inside instead:

function get_directory_contents($directory_list){
    $current_dir = '../../../TestSiteDataFiles/Quotes/';
    $dir = opendir($current_dir);

    // Define $directory_list as an array IN HERE
    $directory_list = array();

    //reads and outputs the directory
    while(false !== ($file = readdir($dir)))
    //strip out the two entries of . and ..
    if($file != "." && $file !=".."){
        array_push($directory_list, $file);
    }
    closedir($dir);
    return $directory_list;
}

This was only really a problem because you used array_push() instead of the [] array append notation. array_push() must take an existing array as its first param, and it won't create one itself.

// Would have worked since the array would get initialized if it didn't exist in scope already.
$directory_list[] = $file;

Answer 2

I think you will find you have a mismatch of variable names in your upload function.

At the start you have $_FILES['userfile']['name'] but in the move_uploaded_file function you use $_FILES['tmp_name']

Maybe check the variables contain what you expect. You will probably find they are evaluating to an empty string, that when concatenated with the paths you are setting results in a directory being set.

Edit: Actually, you are also passing a variables called $_FILES into your upload_quotes function, however $_FILES is a super-global and therefore is available in all scopes (i.e. anywhere). Renaming the argument of upload_quotes (and any reliant variables accordingly) may solve your problem.