Incorrect handling of file upload limit exceeded - file widget disappears

updated issue for clarity

Summary: I could not reproduce the original problem. The file attach widget was available to me after I uploaded a file that exceeded the max size, and it was easy to upload a different file. However, the error message from my first attempt persisted after I successfully uploaded a smaller file. I don't think users need to continue to see the error message once a new file is successfully uploaded. (see attached screenshot)

Test process:
1) Created a file upload field. Set max file size to 40KB.
2) Attempted to upload a file larger than 40 KB.

Results:
I got an error message "The specified file Why Drupal 7.docx could not be uploaded. The file is 92.84 KB exceeding the maximum file size of 40 KB." The file upload widget (Browse button, Upload button) remained visible. I didn't have to refresh my browser.

Next, I successfully uploaded a new, smaller file.

However: the error message from my first attempt to upload a file was still on the screen. I don't think users need to continue to see the error message once a file is successfully uploaded. (see attached screenshot)

Marked #1619842: Field upload widget disappears when file is bigger than PHP limit as duplicate of this.

There are two different messages and file upload limits:

1. If the uploaded file is larger than the limit configured in the content type, but smaller than the upload limit configured in php.ini, you will get the message "The specified file %name could not be uploaded. The file is %filesize exceeding the maximum file size of %maxsize." This message is in file.inc, in two parts.
2. If the uploaded file is larger than the upload limit configured in php.ini, you will get the message, "An unrecoverable error occurred. The uploaded file likely exceeded the maximum file size (@size) that this server supports." This message is in file.module.

In this second case only, the upload wiget disappears and any other file already uploaded also disappears. If you save at this point, any other files uploaded will be removed from the node. I am seeing this in D7.22.

However: the error message from my first attempt to upload a file was still on the screen. I don't think users need to continue to see the error message once a file is successfully uploaded. (see attached screenshot)

This is a duplicate of #1792032: File validation error message not removed after subsequent upload of valid file.

After reading the fix to #1792032: File validation error message not removed after subsequent upload of valid file, I think this is the problem: In file_ajax_upload(), the first call to ajax_command_replace() replaces everything with the error message. The calls to ajax_command_replace() later in file_ajax_upload() include the rendered version of the form in the replacement, so the form still shows.

Here is a D7 patch. I replaced the call to ajax_command_replace() with a call to ajax_command_prepend(), so the error message gets prepended without disturbing the form.

D8 version.

Sorry, my D8 testing box is not working now. I just did what I thought was equivalent.

The failures do not appear to be related to anything changed by the patch. Trying again in case it was a problem with the testing process.

I didn't mean to change the tagging.

Fixing tags.

Fixing tags.

#8: core_1489692_file_upload_ajax_message_D8.patch queued for re-testing.

Reroll.

#17: core_1489692_file_upload_ajax_message_D8.patch queued for re-testing.

#17: core_1489692_file_upload_ajax_message_D8.patch queued for re-testing.

#17: core_1489692_file_upload_ajax_message_D8.patch queued for re-testing.

D8 code has been completely rewritten, removing the function that was patched, but the issue remains in D7.

#22: core_1489692_file_upload_ajax_message.patch queued for re-testing.

edited some formatting

22: core_1489692_file_upload_ajax_message.patch queued for re-testing.

22: core_1489692_file_upload_ajax_message.patch queued for re-testing.

I confirm the patch. It's better to prepend error instead of relapsing the whole file widget.
It should be changed.
Thank you, Liam.

Restoring previous status.

omg.
@Liam, maybe you need to re-roll it with last dev version?

Here is a reroll, but I don't think that is the problem. The errors seem to be unrelated to the patch and it passes some of the time.

Restoring previous status.

Hi,

This patch worked for me.

Thanks

Are we really sure this doesn't affect Drupal 8? I see what looks like equivalent code in core/modules/file/src/Controller/FileWidgetAjaxController.php:

    if (empty($request_form_build_id) || $form_build_id !== $request_form_build_id) {
      // Invalid request.
      drupal_set_message(t('An unrecoverable error occurred. The uploaded file likely exceeded the maximum file size (@size) that this server supports.', array('@size' => format_size(file_upload_max_size()))), 'error');
      $response = new AjaxResponse();
      $status_messages = array('#theme' => 'status_messages');
      return $response->addCommand(new ReplaceCommand(NULL, drupal_render($status_messages)));
    }

Also, if it's true that this one case should be ajax_command_prepend() but the other two calls later in the function should remain ajax_command_replace(), this is screaming for a code comment explaining why :) I would at least think that if this one needs to be ajax_command_prepend() then the one after it should be too...

I don't know if it effects D8 or not. It looked to me like the code had been rewritten, so I moved this to just being a D7 issue.

I only noticed the problem in the place that I fixed it. It seems reasonable that they should all be the same. I just haven't run into the problem with the other code.

Re-roll of D8 patch.

It looks like the old testing system is saying "Queued for testing", but it has passed the new tests, so it should be OK.

#@47: ReplaceCommand() no longer appears in FileWidgetAjaxController.php.

How can I see which test assertion failed?

I am no longer able to reproduce this problem on either D8 or D7. Thanks for whomever fixed it in some other issue.

I can still reproduce this in both Drupal 7 and Drupal 8. Just add a file field to the Basic Page content type, make it accept .gz files for upload, then create a basic page and try to upload some gigantic .gz file that's bigger than the server limit. The error message is displayed, but the file widget disappears from the screen.

Are there more than one upload limit, such as one in Drupal and one in PHP? Maybe that is why the problem appeared to vanish for me.

I believe I tested the scenario where the only upload limit was the PHP one.

David, that sounds familiar. It could be that the problem happens when the PHP limit is reached, but not the Drupal limit.

Rerolled and included the proper class.

Add test and patch.

#68 It works for me.

Thanks!

@juanramonperez Thanks for manually testing this!

I'm (at devdays) and going to do a code review.

I also sent the previous patch to test against 8.2.x also. :)

@josmera01 thanks for changing that test.

Uploading a test only patch, to verify the test will fail without the fix.

(I have the thought that we might want to try in a test to upload a file over the limit, and then assert the error is appropriate, and that the widget is there (and the previously uploaded files), and that another file can be uploaded after the error, and then the error disappears... need to look into it more and think more)

@Liam Morland Thanks so much for the detail (in #3) about the php limit vs the drupal file size limit. Adding that to the summary to keep that information easy to find.

I think this is needs work for a test that asserts the correct behavior (and the absence of the wrong behavior).

Fix for use of current tag. Sorry for using the wrong one.

Also fix usability tag.

Patch in #68 confirmed as working. Is it possible to get this fix in, and open a new issue for adding proper tests?

Fixing #68 in Drupal 7 also solves the problem there.

#32 works buts, if i upload the file again, the error message doesnt desappear

Patch in #68 still relevant & applies cleanly to 8.3.x

Patch in #68 still relevant & applies cleanly to 8.3.x

- I can confirm that. But I can also confirm, that there is a duplicate of the message, when I upload the file again.

It is safe to raise this to major based generally on 'Interfere(s) with normal site visitors' use of the site (for example, content in the wrong language, or validation errors for regular form submissions), even if there is a workaround.'

I cannot replicate the duplicating of the error messages in #87. The first time I try to upload a file that is too large I get the validation message correctly. Uploading the file a second time causes the message to stay visible, it is not duplicated. If I then upload the correct file the message disappears. It all seems to work as expected.

Reading back from the comments this still needs better test coverage.

OK I can replicate the duplicated error message now, the trick is to leave the upload limit empty in the field configuration, and then upload a file that exceeds the PHP limit. This duplicated error message is out of scope for this issue though, it is being handled in #2346893: Duplicate AJAX wrapper around a file field.

However I seem to be unable to replicate the original problem from the issue summary. I tried both the file field and the image field, and different combinations of exceeding the field's file size limit or PHP's file size limit, but the field seems to be working as expected.

I am finally able to replicate this bug. These are the steps needed to replicate it:

• Turn off display_errors in php.ini. If this is on a PHP warning "POST Content-Length of x bytes exceeds the limit of y bytes" will be present at the top of the AJAX response, and this prevents the AJAX payload from being executed.
• Make sure the post_max_size is set to equal or lower than the upload_max_filesize in php.ini. The bug only manifests itself when the post_max_size is exceeded.
• Upload any file larger than the post_max_size. The file field will be replaced with the error message.

Will update issue summary with this additional information.

Added the Javascript test suggested by @YesCT in #76.

The patch in #93 worked for me. Test also seems to be quite complete.

#68 works for me

#68 is outdated, the current patch is #93 for 8.4.x.

@geekinpink, you probably need this for Drupal 8.3.x? We could backport the current patch to 8.3.x but I don't know if it is worth the effort since 8.4.x is already in RC phase and will get a full release within a month. Any volunteers? :)

#93 worked for me; Drupal version 8.3.6

#93 worked for me, drupal version 8.4.3

Committed/pushed to 8.5.x and cherry-picked to 8.4.x. Thanks!

We are still seeing this issue on D7 (or maybe seeing it again?). The patch from #7 still works for us against 7.

Backporting the fix to D7 is being done in #2749245: Back port: Incorrect handling of file upload limit exceeded - file widget disappears.