2
\$\begingroup\$

I am trying to make a webpage for uploading an image file and storing the uploaded file on server. There are several features in this implementation:

  1. There is a drag & drop zone on the webpage. You can drag the file to be uploaded to this zone and drop. Also, you can click this zone and select the file to be uploaded.

  2. After selecting a file, you can check the selected filename on the webpage.

  3. Finally, you can click "Upload To Server" button to perform the upload process.

  4. There is a progress bar for displaying the upload progress.

  5. On the server side, the uploaded files are placed in folder /var/www/html/uploads/ and filenames are updated with $date_time prefix.

The experimental implementation

  • index.html: main page for selecting and uploading file

    <!DOCTYPE html>
    <html>
    
    <head>
        <style>
            html {
                font-family: sans-serif;
            }
    
            div#drop_zone {
                height: 400px;
                width: 400px;
                border: 2px dotted black;
                display: flex;
                justify-content: center;
                flex-direction: column;
                align-items: center;
                font-family: monospace;
            }
    
        </style>
    
    </head>
    
    <body>
        <h2>Upload File Page</h2>
        <!╌ https://stackoverflow.com/a/14806776/6667035 ╌>
        <input type="file" name="file_to_upload" id="file_to_upload" accept=".bmp, .jpg, .png" style="display: none;" multiples>
        <h3>Drag & Drop a File</h3>
        <div id="drop_zone">
            DROP HERE
        </div>
        <hr>
        Selected filename: 
        <p id="file_name"></p>
        <progress id="progress_bar" value="0" max="100" style="width:400px;"></progress>
        <p id="progress_status"></p>
        <input type="button" value="Upload To Server" id="upload_file_button">
    
        <script type="text/javascript">
            document.getElementById('file_to_upload').addEventListener('change', (event) => {
                window.selectedFile = event.target.files[0];
                document.getElementById('file_name').innerHTML = window.selectedFile.name;
            });
    
            document.getElementById('upload_file_button').addEventListener('click', (event) => {
                //  Reference: https://stackoverflow.com/a/154068/6667035
                if (document.getElementById('file_name').innerHTML === "") {
                    //  Reference: https://stackoverflow.com/a/10462885/6667035
                    var check = confirm("Please select a file!");
                    if (check == true) {
                        return true;
                    }
                    else {
                        return false;
                    }
                } else {
                    uploadFile(window.selectedFile);
                }
            });
    
            const dropZone = document.getElementById('drop_zone'); //Getting our drop zone by ID
            dropZone.addEventListener("click", drop_zone_on_click); //  Regist on click event
            if (window.FileList && window.File) {
                dropZone.addEventListener('dragover', event => {
                    event.stopPropagation();
                    event.preventDefault();
                    event.dataTransfer.dropEffect = 'copy'; //Adding a visual hint that the file is being copied to the window
                });
                dropZone.addEventListener('drop', event => {
                    event.stopPropagation();
                    event.preventDefault();
                    const files = event.dataTransfer.files; //Accessing the files that are being dropped to the window
                    window.selectedFile = files[0]; //Getting the file from uploaded files list (only one file in our case)
                    document.getElementById('file_name').innerHTML = window.selectedFile.name; //Assigning the name of file to our "file_name" element
                });
            }
    
            // ---- function definition ----
            async function drop_zone_on_click(){
                //document.getElementById('input_file').click();
                let files = await selectFile("image/*", true);
                window.selectedFile = files[0];
                document.getElementById('file_name').innerHTML = files[0].name;
            }
    
            //  Reference: https://stackoverflow.com/a/52757538/6667035
            function selectFile (contentType, multiple){
                return new Promise(resolve => {
                    let input = document.createElement('input');
                    input.type = 'file';
                    input.multiple = multiple;
                    input.accept = contentType;
    
                    input.onchange = _ => {
                        let files = Array.from(input.files);
                        if (multiple)
                            resolve(files);
                        else
                            resolve(files[0]);
                    };
    
                    input.click();
                });
            }
    
            function uploadFile(file) {
                var formData = new FormData();
                formData.append('file_to_upload', file);
                var ajax = new XMLHttpRequest();
                ajax.upload.addEventListener("progress", progressHandler, false);
                ajax.open('POST', 'uploader.php', true);
                ajax.send(formData);
            }
    
            function progressHandler(event) {
                var percent = (event.loaded / event.total) * 100;
                document.getElementById("progress_bar").value = Math.round(percent);
                document.getElementById("progress_status").innerHTML = Math.round(percent) + "% uploaded";
            }
        </script>
    </body>
    
    </html>
    
  • uploader.php: performing the storing files in the back-end.

    <?php
    $file_name = $_FILES["file_to_upload"]["name"];
    $uploaddir = '/var/www/html/uploads/';
    //  Reference: https://stackoverflow.com/a/8320892/6667035
    //  Reference: https://stackoverflow.com/a/18929210/6667035
    //  Reference: https://stackoverflow.com/a/27961373
    $time = date("Y-m-d_H-i-s_");
    $file_name_on_server = $uploaddir . $time . basename($file_name);
    $file_temp_location = $_FILES["file_to_upload"]["tmp_name"]; 
    if (!$file_temp_location) { 
        echo "ERROR: No file has been selected";
        exit();
    }
    
    if ($_FILES["file"]["error"] > 0) 
    { 
        echo "Return Code: " . $_FILES["file"]["error"] . "<br />";
        exit();
    } 
    
    if(move_uploaded_file($file_temp_location, $file_name_on_server)){
        echo "$file_name upload is complete";
        //  Reference: https://www.php.net/manual/en/function.system.php
        $last_line = system('ls', $retval);
    } else {
        echo 'Error code' . $_FILES['file_to_upload']['error'] . '<br/>';
        echo "A server was unable to move the file";
    }
    return NoContent();
    ?>
    

The webpage screenshot

ImageFileUploaderWebpage

All suggestions are welcome.

\$\endgroup\$
2
  • 1
    \$\begingroup\$ You forgot to mention that it has a progress bar. Code looks fine to me. You could consider putting the Javascript in a separate file and make it reusable. Next up: Do something with the uploaded file. \$\endgroup\$ Commented Feb 27, 2022 at 9:40
  • \$\begingroup\$ @KIKOSoftware Thank you for the suggestion. \$\endgroup\$
    – JimmyHu
    Commented Mar 1, 2022 at 0:00

1 Answer 1

3
\$\begingroup\$

Back end code

Add file type checking

As this comment on PHP.net mentions:

A note of security: Don't ever trust $_FILES["image"]["type"]. It takes whatever is sent from the browser, so don't trust this for the image type. I recommend using finfo_open (http://www.php.net/manual/en/function.finfo-open.php) to verify the MIME type of a file. It will parse the MAGIC in the file and return it's type...this can be trusted (you can also use the "file" program on Unix, but I would refrain from ever making a System call with your PHP code...that's just asking for problems).

The front-end code uses the accept attribute on the file input, however that does not prevent a user from submitting the form with a file of another type. A user could modify the HTML using a browser console or change the file selector interface to show all files instead of image files - see the animation below for a demonstration. I was able to change the interface to allow selecting a file of any type both in Mac OS and Windows.

changing file type selection

The request to the back-end could also be made from another page or a tool like cURL, Postman, etc.

finfo_open() can be used to ensure the uploaded file is an image, though there is a simpler solution in this review using exif_imagetype().

Unused variables can be removed

The implementation of NoContent is not included so we can only guess at what it does. Unless it uses $last_line or $retval then those variables can be eliminated.

Brace styles are inconsistent

There are three if statements. The block for the second one has the curly brace starting on a new line. Perhaps this is a familiar pattern in other programming languages but idiomatic PHP typically only has braces on the same line after an if statement. One is not required to follow a style guide but a popular one for PHP is PSR-12. Section 5.1 covers if statements.

Front end code

Please promise to remove excess promises

It is nice that the code uses async and await along with dynamic creation of the <input> element, however that isn't necessary. The click method can be called in the hidden file input. It appears an attempt may have been made to achieve this, given the first line within drop_zone_on_click. That commented line appears to reference an element that does not have an id attribute with value input_file.

A reference could be stored for the hidden file input:

const file_upload_element = document.getElementById('file_to_upload');

Then the click event listener could be added to the drop zone:

dropZone.addEventListener("click", () => file_upload_element.click());

The event listener could also be added using the function bind() method:

dropZone.addEventListener("click", file_upload_element.click.bind(file_upload_element)); 

Then functions drop_zone_on_click and selectFile can be eliminated.

Logic can be simplified

Above the line where check is declared in the event handler callback for the element with id upload_file_button there is a reference to a Stack Overflow answer:

           //  Reference: https://stackoverflow.com/a/10462885/6667035
           var check = confirm("Please select a file!");

That answer has multiple comments.

You can replace if(check == true) with if(check). – Anonymous Apr 1, 2020 at 21:05

please just return confirm("Are you sure you want to leave?"); and save 6 lines. – Florian F Jun 15 at 12:35

Both comments are valid.

Then again the return value from the call to confirm is not really used.

One could simply use alert() instead of confirm(), then simply have a return statement with no return value.

As explained in this review some users could disable alerts in a browser setting so a different technique like using the HTML5 <dialog> element.

Attribute multiples can be removed from file input

The file input has an attribute multiples specified. Such an attribute does not exist though there is a multiple attribute that could be applied. The code simply uploads a single file so it does not seem like there is any use for that attribute.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.