getgrav / grav-plugin-form

Grav Form Plugin
http://getgrav.org
MIT License
53 stars 79 forks source link

PHP file upload error from backend shows success #428

Open torohill opened 4 years ago

torohill commented 4 years ago

If there is a PHP file upload error when handling a file upload (eg. https://www.php.net/manual/en/features.file-upload.errors.php) the frontend still shows success.

Reproduction steps:

  1. Create a page with a file upload field.
  2. Change the permissions on PHP's upload_tmp_dir directory to cause a permission error.
  3. Select a file for upload, which gets uploaded via AJAX before the form is submitted.
  4. Note the tick mark that indicates a successful upload.
  5. Use the browser dev tools to see that the response from the server for the POST to task:file-upload was {"status":"error","message":"Unable to upload file filename.txt: "}.

I took a look at the code in the handleError function in app/fields/file.js line 206 and it simple returns true. This code should probably display a failure and show the error message returned from the backend.

Also, the error message returned from the backend doesn't look to be complete. I will submit a pull request to fix this in a bit.

torohill commented 4 years ago

For a little more detail on step 2 from above:

I ended up doing the following to reliably reproduce the error:

ricardo118 commented 4 years ago

this seems like a lot of loops to trigger an error that ive never seen come up? The problems plugin will also error when /tmp isnt writeable and its a default plugin that comes with grav

torohill commented 4 years ago

I had an issue with PHP generating file upload errors in production, in that case it was large file uploads and PHP failing to write to disk. After investigating I saw that the Form class was using $upload_errors when it wasn't defined, and the frontend wasn't displaying the error returned from the backend.

The reproduction steps above are simply the best I could come up with to reliably reproduce the error in a dev environment. Any set of circumstances which causes PHP to generate one of the file upload errors (https://www.php.net/manual/en/features.file-upload.errors.php) will trigger the same behaviour.