After updating to Drupal 7.43 the file component in the webform module failed to upload a picture. After a reset to Drupal 7.42, everything was fine again. Due to the fact that Drupal 7.43 restricts access to temporary files for anonymous user, the error could be probably found in this area .

Comments

ralph_moser created an issue. See original summary.

David_Rothstein’s picture

Priority: Normal » Major
Status: Active » Needs review
StatusFileSize
new4.05 KB

I can confirm this. It happens only when the user clicks "Upload" to upload their file. (If they just choose a file and then use the main form submit button, it works OK.)

I debugged this and it seems the Webform module doesn't set a #default_value for its 'managed_file' form elements correctly. (I guess maybe it never mattered before, but now, in the case of anonymous users, Drupal core is essentially relying on that to know that the file actually belongs there after an Ajax request.)

Here is a possible patch. I am not sure if it has any side effects, but it seems to work in my testing. The part of it which handles multiple file fields on the same form is kind of a hack, but without that part of the patch, if you have multiple file fields on the same webform, only the last file they upload will get saved.

I will add a note about this to https://www.drupal.org/drupal-7.43-release-notes as well.

David_Rothstein’s picture

Version: 7.x-4.12 » 7.x-4.x-dev

By the way, it's worth noting - at least in my testing, there is no feedback to the end user that their file won't be submitted. It looks to them like the upload was successful. That's one reason I marked this as major.

mitchalbert’s picture

any patch for the 7.x-3.24 version?

testii28’s picture

Yes i am looking also for a patch for 7.x-3.24 version

stefan.r’s picture

StatusFileSize
new4.17 KB

Backport to 3.x, can anyone test?

stefan.r’s picture

Version: 7.x-4.x-dev » 7.x-3.x-dev
stefan.r’s picture

StatusFileSize
new4.17 KB

Re-uploading for testbot

The last submitted patch, 6: 2675170-6.patch, failed testing.

calebtr’s picture

Patch at #8 applies to 7.x-3.x-dev and fixes the issue.

awochna’s picture

Status: Needs review » Reviewed & tested by the community

Patch at #2 applies to 7.x-4.12 and fixes the issue for me.

slipperysky’s picture

Will patch #8 work with 7.x-3.24 or do I have upgrade it to 7.x-3.dev? When I tried the patch with 3.24, after submitting the form I got this warning:

Warning: array_flip(): Can only flip STRING and INTEGER values! in DrupalDefaultEntityController->load() (line 175 of /home/aabbcc/subdomains/devs/public_html/includes/entity.inc)

array_flip() is in webform.module on line 3536, but that may not mean anything.
$disabled = array_flip(variable_get('webform_disabled_components', array()));

Also the file URL was not in the email so it didn't work.

A while back upgraded 7.x-3.24 to 7.x.4.x but I can't remember if I had to do anything extra to make it work. Has anyone else recently moved 3.24 to 7.x.4.x without too much trouble? I might try that instead.

stefan.r’s picture

Status: Reviewed & tested by the community » Needs review

Back to NR for #12

stefan.r’s picture

@slipperysky #8 ought to work with 7.x-3.24, but maybe it doesn't considering your error - do you have any way to reproduce the issue in a clean webform 3.x + drupal 7.43 install?

For instructions on upgrading to 4.x see https://www.drupal.org/node/1609324

criscom’s picture

Version: 7.x-3.x-dev » 7.x-4.12
Status: Needs review » Fixed

Patch #2 applies to 7.x-4.12 and fixes the issue also for me. Thanks for the fast patch!

stefan.r’s picture

Status: Fixed » Needs review

Back to NR for #12

ralphmoser’s picture

Thank you very much for the very fast reaction on this issue. I can confirm that the patch #2 perfectly solves the problem for webform 4.12.
On the other hand for webform 3.24 both patches #2 and #8 didn't work for me.

slipperysky’s picture

@stefan.r - I will check if I can reproduce the same error with a clean install. Thanks for the link. I may end up upgrading to 4.x even if there's a working patch for 3.24.

amaisano’s picture

Status: Needs review » Needs work

There is a problem with patch #2, or a scenario that is incompatible with it. Files selected but not explicitly uploaded -- in other words, selecting a file, then using the main webform submit button to submit everything -- do NOT get saved if the form throws a validation error (for instance, invalid email address) and reloads the form. When the form reloads, the file HAS been uploaded and is present in the file component, but upon fixing the form error and hitting the webform submit button again, the file is NOT saved / no longer attached to the form submission.

I compared the behavior between 7.42 and 7.43 + webform patched. Can anyone else confirm?

1) Create a webform with Name (required), Email (required), and File (optional - NOT required)

2) Log out, and visit the webform anonymously

3) Enter a name, SELECT a file (but don't hit upload)

4) For email, type "name@x" and hit submit - we want to trip a validation error and have the page reload

5) Notice the file is now attached/uploaded.

6) Fix the email by typing "name@x.com" and hit the main submit button for the webform

7) Log back in and check the webform submission and review the file field - no file!

slipperysky’s picture

@stefan.r - I can only reproduce half of the issue I experienced on the original form. On a clean install the warning (from #12) is not present, but the files are still not showing up in the emails so it doens't seem to be working in a clean install with 3.24. It's a simple form with 2 file uploads. The original form where I also see the warning is much larger with multiple file uploads so possibly there is something else that is helping to create the warning. Everything is working fine if I go back to Drupal 7.42.

santhoshabraham’s picture

I'm having same issue with our site. I have 2 file upload, none of the shows on e-mail or database

testii28’s picture

I have applied the patch 4.12 and 3.24 and I am using three file fields to upload documents. but it only uploads the last file, the others not. Can that be fixed too? If that option doesn´t work both drupal site I have developed will be useless.

Thanks

stefan.r’s picture

#22 I cannot reproduce, but #19 yes (and #19 we should definitely address in this patch)

stefan.r’s picture

Version: 7.x-4.12 » 7.x-3.x-dev
StatusFileSize
new4.78 KB

New 7.x-3.x patch which should fix the issue in #12

@David_Rothstein or others, both this patch (#24) and the patch in #2 still have the issue in #19 -- what would be the best way around that? Ideally we'd set the right fid even if validation fails so people don't have re-upload their file.

slipperysky’s picture

I set Webform back to 3.24 under Drupal 7.43 and used the new patch in #24.

It looks like the warning I was seeing in #12 is gone, but I'm not sure if that was meant to get fixed here.

Unfortunately it looks like I am now experiencing what happened in #22. I'm testing a form that has 5 file fields. If I only upload the first file, it will show up in the email, but if it is more than one file, only the last uploaded file will make it into the email.

dalanz’s picture

Same as #25 with 5 Fields to upload after applying #24. It only happens on new sessions. As soon as you reload the page. It works with all files. The Error I get back on uploading another file on new sessions is about a too big file (which is not the case).

stefan.r’s picture

Can anyone provide a way to reproduce the issue in #22/#24/#26 on a clean Drupal/webform install? If I create a webform with multiple file fields, with the patch it still submits both files, so I cannot reproduce this.

nicrodgers’s picture

Is @ralph_moser actively working on this, or can it be unassigned?

stefan.r’s picture

Assigned: ralphmoser » Unassigned
nicrodgers’s picture

I've been trying to reproduce the error described in #22 and #26, on a clean install of 7.43 with webform-7.x-3.x.

I've been able to replicate an issue where selected files are lost, but I'm not sure if this is the same thing as described in #22. It happens with and without the patch from #24 applied. I can also replicate it using a clean install of 7.42 and webform-7.x-3.x, suggesting that it's not directly related to this issue.

1. Clean install D7.43 with webform-7.x-3.x
2. Add a webform with multiple file upload fields
3. log out and go to the webform as an anonymous user
4. open your javascript console
5. Click select file on one of the file inputs, and select a file. Don't press upload.
6. Click select file on another of the file inputs, and select a file. Don't press upload.
7. Observe the javascript errors in the console "Uncaught TypeError: Cannot read property 'length' of undefined" file.js "if (extensionPattern.length > 1 && this.value.length > 0) {"
8. Click upload next to the first file, then very quickly click upload next to the second file.

Result: First file is uploaded, but second file input reverts to the 'select file' state.

Note that you have to press the second upload button very quickly after pressing the first, otherwise you wont replicate the issue. I've tried quite a few times and can reproduce this consistently, but it all depends on you clicking the upload buttons in quick succession.

If this is how to replicate #22 and #26 then I'd suggest creating a new issue for it, and proceed with the patch from #24 (#19 still needs addressing), because neither the patch, nor the update to drupal 7.43 is responsible.

Here's a screen recording of me reproducing what I describe above:
https://www.dropbox.com/s/3dupbsxmbquut2d/webform.mov?dl=0

lidiia.litovko’s picture

I have the same issue with drupal 7.43, Webform 7.x-4.12 and Webform Ajax 7.x-1.1. Anonymous users can upload file on the server, but they can't send form after click button Send because file is missing.

damienmckenna’s picture

Status: Needs work » Needs review

Triggering the testbots for the patch in #24.

damienmckenna’s picture

I suggest adding more tests to cover the scenario detailed in comment #19.

damienmckenna’s picture

Version: 7.x-3.x-dev » 7.x-4.x-dev

The standard practice is to work on the latest branch and backport the final fix.

damienmckenna’s picture

As a baseline, I'm unable to reproduce the problem mentioned in #34 using Drupal 7.42 and the latest Webform 7.x-4.x using an otherwise barebones Standard site installation.

damienmckenna’s picture

(Sorry, I just realized that the error mentioned in #30 is related to 7.x-3.x, not 7.x-4.x)

damienmckenna’s picture

Status: Needs review » Reviewed & tested by the community

Following standard practices, the patch in #2 fixes the issue for the 7.x-4.x branch, so I've marked this as RTBC. Lets get this fixed first and then worry about the (deprecated) 7.x-3.x branch.

stefan.r’s picture

So #19 was a separate issue and was also broken before 7.42?

Also #22/#31 mentioned some issues with the 4.x patch, those have been addressed?

damienmckenna’s picture

Given the simpletest limitations with Javascript, is this even testable?

damienmckenna’s picture

Status: Reviewed & tested by the community » Needs work

I've re-tested it and yes, triggering a validation error does still trigger the error.

Steps to reproduce:

  • Create a webform with two components: 1. a test field that is required, 2. a file field that is not required.
  • Anonymously, add a file to the form but do not fill in the text field. Click "Submit".
  • The form will reload and indicate that the required text field is empty; Note that the file field indicates that the file was uploaded.
  • Fill in a string into the text field and click 'upload'.
  • The submission will be in the database, but the file will have disappeared.
slipperysky’s picture

Wanted to add a little more info based on #25. I had went directly to the original site to test the newest patch in #24 and got those results in #25. Soon after that I was able to test a clean install (7.43/3.24) with 2 file fields and I couldn't reproduce it.

But at the same time, there are at least a couple people commenting here who seem to be having the exact problem that I'm having with an "unclean" install, where only the last file field makes it into the email.

Edit: Never mind the PDO exception below. I found out that the database was somehow getting out of sync with the actual files uploaded to the server so that's probably what caused the error.

Plus, something new started happening. I went back to test the original site again and realized that I hadn't cleared the cache, so I did that and then I started getting an AJAX PDOException for trying to upload a duplicate file.
PDOException: SQLSTATE[23000]: Integrity constraint violation: 1062 Duplicate entry
This error only seems to happen on the file fields after the first one. I can successfully upload a duplicate file that gets renamed on the first field, but after that the other file fields will give the error.

Maybe it's the setup in this particular form, but I'll keep testing to make sure this is what's really happening.

EDIT: I tested uploading each file field in random order and started getting random results. Sometimes the PDO error was triggered, sometimes not. It also started getting triggered by the first file field so that was wrong. I've never seen this error before in Drupal so I'm not sure what to think of it. Sorry if it's going off in a direction that's not related to the reproducible issues.

damienmckenna’s picture

Assigned: Unassigned » damienmckenna
Status: Needs work » Needs review
StatusFileSize
new12.01 KB

I'm working on some tests for the file handling; these aren't finished yet but I wanted to upload something.

Status: Needs review » Needs work

The last submitted patch, 42: webform-n2675170-42.patch, failed testing.

damienmckenna’s picture

BTW the test in #42 depends upon #2678026: Tidy up the tests a little.

damienmckenna’s picture

Status: Needs work » Needs review
StatusFileSize
new26.06 KB

I've expanded the tests to cover actual file uploading.

Status: Needs review » Needs work

The last submitted patch, 45: webform-n2675170-45.patch, failed testing.

damienmckenna’s picture

Status: Needs work » Needs review
StatusFileSize
new8.43 KB
new15.74 KB

This adds tests to confirm file uploads still work when a required text field is initially left empty but then submitted again.

Status: Needs review » Needs work

The last submitted patch, 47: webform-n2675170-47.patch, failed testing.

damienmckenna’s picture

Status: Needs work » Needs review
StatusFileSize
new4.31 KB
new17.23 KB

This tests multiple file uploads, and here's the output so you don't think I'm crazy ;-)

Drupal test run
---------------

Tests to be run:
 - Webform file submission (WebformFileSubmissionTestCase)

Test run started:
 Monday, February 29, 2016 - 17:25

Test summary
------------

Webform file submission 256 passes, 0 fails, 0 exceptions, and 54 debug messages

Test run duration: 2 min 43 sec

Status: Needs review » Needs work

The last submitted patch, 49: webform-n2675170-49.patch, failed testing.

damienmckenna’s picture

Status: Needs work » Needs review
StatusFileSize
new64.64 KB

This patch includes the changes from 2678026, just so you can see how the tests work.

damienmckenna’s picture

So... the tests pass, even though I haven't changed the code *and* it tests the scenario that fails in manual testing. Is this a limitation of the tests or Simpletest?

stefan.r’s picture

Priority: Major » Critical

Critical because of data loss of submitted uploads (without any error message indicating so)

therobyouknow’s picture

I have three questions, if you could advise please:

1) Looking at: https://www.drupal.org/node/2675170 Can you advise which patch should be used:

Are they identical in terms of fix (but #51 contains test code?)

2) Is this correct: These patches will fix the issue in webform 3 if they are applied to the latest version of Webform 3, i.e. 3.24 (as of 2016-03-01) ?

3) When applying the patch, can the following command be used:
patch -p1 < webform-n2675170-49.patch
when cd into in the webform module folder sites/all/modules/contrib/webform/ ? Or would you recommend another approach to apply the patch?

Thank you very much for providing the fix.

damienmckenna’s picture

@therobyouknow: 1) Neither #49 nor #51 are for Webform 7.x-3.x, they're for the 7.x-4.x branch. 2) No, you need patch #24 for 7.x-3.x but that still has problems. 3) You can skip having to manually download the file using this command: curl URLTOTHEPATCH | patch -p1 That skips downloading the file.

therobyouknow’s picture

Thank you very much @DamienMcKenna for following up with the information and so soon too, most appreciated.
From #19 it would appear then that this issue is that an uploaded file associated with a webform submission becomes orphaned after re-submitting a form following correcting other invalid fields, have I understood correctly?
Just to confirm #24 is only for 3.24 (latest, currently) is that right? (You mentioned 3.x, so I'm assuming it is the latest version, or is it for 3.x-dev?)
Thanks very much again for all your input.

therobyouknow’s picture

Answering my last question:
Yes - #24 indeed IS for webform-7.x-3.24 (the latest version of webform 3 currently).
I know this because the patching works and I inspected it in diff and Beyond Compare (before and after) comparing the diffs shown on these tools with the patch itself. The curl command works perfectly - thank you so much!
I'll test the solution.
I'm inclined to go with what we have here and perhaps add a markup in the form to warn users that they need to re-upload their files if they had invalid fields elsewhere. But I will watch the developments on this issue with interest as well and of course any help I can give I would try to. Thanks!

damienmckenna’s picture

The fun part of patch #51? The testAnonFileRequiredSubmission test should fail, but it doesn't. Not sure if this is a limitation of the tests, of Simpletest, or the module itself.

santhoshabraham’s picture

Hi,
I applied patch #24 IS for webform-7.x-3.2, and still having issue uploading multiple file for anonymous user. It works fine if you log in.

Here is the steps you can duplicate.

This is for version 7.x-3.24

1. Install D7.43 with webform-7.x-3.24
2. Add a webform with multiple file upload fields and make it REQURED field.
3. log out and go to the webform as an anonymous user
4. Upload files to both field
5. Click submit.

Now you going to see only one file upload and one file field with error message.

David_Rothstein’s picture

Regarding #19, nice find, but I'm able to reproduce that with Drupal core also, so it's probably not something for Webform to fix. I created an issue about it in the Drupal core queue now.

webservant316’s picture

#50 works for me, though I manually applied the patch to webform.module and file.inc without the patches to the tests. #49 and #50 are identical except for differences in the patches to the tests. I wasn't sure which was recommended so I just patched what was needed for the fix.

Thanks for these patches!

David_Rothstein’s picture

The fun part of patch #51? The testAnonFileRequiredSubmission test should fail, but it doesn't. Not sure if this is a limitation of the tests, of Simpletest, or the module itself.

Maybe try changing fileRequiredSubmissionTest() so that the second submission at the end, i.e. this part:

+    // Submit the form again but this time with the missing field.
+    $edit['submitted[textfield]'] = 'Test submission';
+    $this->drupalPost('node/' . $nid, $edit, 'Submit', array(), array(), 'webform-client-form-' . $nid);

does a drupalPost() to NULL rather than to 'node/' . $nid, $edit? (That way it will submit the existing form again rather than do a GET request in between to reload the form.)

damienmckenna’s picture

StatusFileSize
new750 bytes
new17.39 KB

This changes the fileRequiredSubmissionTest() to resubmit the current page instead of submitting to the specific URL again, per David_Rothstein's suggestion. Also, duh Damien :p

Status: Needs review » Needs work

The last submitted patch, 63: webform-n2675170-63.patch, failed testing.

amaisano’s picture

Thank you, David, for confirming and discovering the source of #19!

cjallenD1’s picture

Thanks David! I applied these patches and at first tests all seems well. I am using Webform 4.12

I have two file uploads for an application and these two files accept doc/pdf.

I have combined the patch from #2 in combination with the AutoUpload module and it seems to be operational.

I'll continue to test and post updates.

cjallenD1’s picture

StatusFileSize
new20.74 KB

I get the following now when uploading multiple files with the patch in #63

An unrecoverable error occurred. The uploaded file likely exceeded the maximum file size (20 MB) that this server supports.

The file in question is well below 20mb (22kb).

damienmckenna’s picture

I've uploaded a patch to #2678822: Drupal 7.43 / 8.0.4 regression: When an anonymous user submits a form with an un-uploaded file that leads to a validation error, the file is lost on the next correct submission that shows the error occurs with a standard node submission by an anonymous visitor, like David Rothstein indicated.

damienmckenna’s picture

@cjallenD1: That's.... really weird. Is the problem consistent? Is it reproducible across multiple webforms, or just one?

damienmckenna’s picture

Assigned: damienmckenna » Unassigned

Leaving this for someone else for now, need to do some other things for a while.

cjallenD1’s picture

DamienMcKenna - I just ran a few more tests and believe that 'Unrecoverable error .. file size ..' error was due to the combination of the patch + the module AutoSubmit.
Upon disabling AutoSubmit and hiding the upload button on my webform via CSS the uploads work properly.

-chris

galactus86’s picture

Glad to see this issue is being looked at. I am using version 7.x-3.24. When anonymous user submits a webform with upload the file goes to the server. But when I check the results page, the link to the resume is not there. This seems similar to what this issue is discussing. Does patch #24 work?

beezer75’s picture

Same issue here. The files seem to upload fine for unauthenticated users who bypass clicking upload and go straight to submitting the form. They disappear if the user clicks upload. The functionality is fine for authenticated users.

webservant316’s picture

I posted above in #61 saying that the patches in #49 and #50 work for me with Drupal 7.43 and Webform 7.x-4.12. I manually applied the patch and skipped the part of the patch dealing with regression testing.

Anyway I am trying to follow the post discussion to understand if all is well with the patches in #49 and #50. I was concerned and so did further testing and in my use case everything still works fine when an anonymous user uses the file upload button or simply clicks the submit button only at the bottom of the form. My use case only allows the upload of one file.

Just reporting back.

damienmckenna’s picture

@webservant316: Have you tested what happens if the visitor triggers a validation error, e.g. fails to fill in a required field? In that scenario a bug in core will loose the file.

webservant316’s picture

ok - just tested that.

So as an anonymous user I used webform to upload a single file using the form submit button with a validation error, corrected the error, submitted and the file was there just fine. I then did it again using the upload button, submitted with a validation error, fixed the error, submitted and again the file was there just fine.

I am using Drupal 7.43 and Webform 7.x-4.12 with the patch from #49 and #50 manually applied without the regression testing part of the patch.

cjallenD1’s picture

Damien, webservant316 -

In my use case with Webform 4.12 and patch from #2 I believe, I do get the validation error with invalid email (mail@mail).
When doing this the files show to be there. However, upon submission they get wiped out.
Again, this was from an earlier patch (i think from #2).

If webservant316 does not get that 'validation error/file removal' error with #49 + #50 I will try those patches and report back.

webservant316’s picture

@cjallenD1 keep in mind that patch #49 and #50 are the same with regard to the actual fix, however they each propose different regression testing. Since I didn't know which was better I manually applied pieces of the patch only to webform.module and file.inc.

cjallenD1’s picture

@webservant316 - Oh I see. I did the same; applied only patches to webform.module and file.inc (ignoring tests).

Strange that my files do get wiped out after validation.

webservant316’s picture

hmmm strange. I tested further submitting a webform that returned a validation error without a file attached and then attached a file before the final submit, both with and without using the upload button and that worked for me as well.

I wonder what the difference is? Does the type of validation error matter? I was just leaving a required field blank.

Now my file field is not required. Is that the difference?

amaisano’s picture

For me, it was not putting a domain extension on an email field (person@place instead of person@place.com).

cjallenD1’s picture

@webservant316 - possibly. Both my file upload fields are marked (and need to be) required.

I'll test on my sandbox on required/unrequired fields

calebtr’s picture

I notice that using Mollom to validate webform submissions is breaking file upload. My sense is that this is just related to the validation issue in core and the workaround for now is to disable Mollom.

Any other modules that provide additional form validation for anonymous users could cause the same problem.

webservant316’s picture

@amaisano
A attached a file with upload, entered bad email, submitted, validation failed, fixed email, re-submitted.
Everything was fine.

dherbold’s picture

Been following this.

With patch 49/50 applied, probs still exist.

Clearing browser cache, upload of subsequent files (after 1st file), fails.
Reload page. Do NOT clear cache. Upload of multiple files succeeds.

Clear cache again. Reload page. Upload subsequent files (after 1st file), fails.

David_Rothstein’s picture

I just posted a core patch at #2678822-13: Drupal 7.43 / 8.0.4 regression: When an anonymous user submits a form with an un-uploaded file that leads to a validation error, the file is lost on the next correct submission that needs testing and may fix all this on its own. (Not just the part that was the core bug, but it may fix the Webform-specific parts also.) So it would definitely be good to test that with Webform.

Making Webform set #default_value correctly (as the patches here try to do) is still a good idea anyway (and of course the tests are a very good idea), but if this regression can be addressed in core then it becomes a lower-priority issue for Webform to have to deal with on its own.

jdflorez’s picture

Tested #87.
Attached two files without, entered bad email, submitted, validation failed, submitted again, validation failed, validated email, re-submitted & the files were uploaded.

Previously, if a validation failed after the files were uploaded, in the next reload the uploaded files were lost.

srutland’s picture

StatusFileSize
new27.5 KB

Applied #49 with the testing code and solves the upload problem for anonymous users. Applied it to our profile which is D7.43 and Webforms 7.x-4.10.

Uploaded an Excel file of our testing results. Let me know if we missed anything.

Also validated the error indicated by David_Rothstein in #87. Will be applying that patch later today.

Found a different problem (for a different issue tix) when multiple file upload fields are used. If a user clicks the upload buttons too fast: the second, fourth, and so on fields clear out and the files don't upload. Validated this is the case with D7.42 as well. Again, this isn't a problem with the current issue.

Thanks to all for the quick response on this issue.

damienmckenna’s picture

Status: Postponed » Needs review
StatusFileSize
new2.55 KB
new17.43 KB

I tested out the latest patch from #2678822: Drupal 7.43 / 8.0.4 regression: When an anonymous user submits a form with an un-uploaded file that leads to a validation error, the file is lost on the next correct submission and it seems to solve the problem.

This patch fixes one item in the tests and they now all pass correctly when that patch is applied to core.

$ simpletest --class WebformFileSubmissionTestCase

Drupal test run
---------------

Tests to be run:
 - Webform file submission (WebformFileSubmissionTestCase)

Test run started:
 Thursday, March 3, 2016 - 16:26

Test summary
------------

Webform file submission 252 passes, 0 fails, 0 exceptions, and 52 debug messages

Test run duration: 2 min 27 sec
damienmckenna’s picture

FYI all tests pass even without the changes to components/file.inc or webform.module:

$ simpletest --class WebformFileSubmissionTestCase

Drupal test run
---------------

Tests to be run:
 - Webform file submission (WebformFileSubmissionTestCase)

Test run started:
 Thursday, March 3, 2016 - 16:30

Test summary
------------

Webform file submission 252 passes, 0 fails, 0 exceptions, and 52 debug messages

Test run duration: 2 min 34 sec
damienmckenna’s picture

Status: Needs review » Needs work

The last submitted patch, 91: webform-n2675170-90.patch, failed testing.

slipperysky’s picture

I upgraded Webform 7.x3.24 to 7.x4.12. Then I tried applying anonymous-file-upload-2678822-13.patch to Drupal and applied the patch in #49 (without the tests). I submitted the form with 3 file uploads with no problem. All the file links showed up in the email. I tried it again triggering validation with no problems. Was there another issue I missed?

So far for me it looks like all the original issues have been fixed.

srutland’s picture

@izmeez, continuing from #89 above we have now patched with #2678822: Drupal 7.43 regression: When an anonymous user submits a form with an un-uploaded file that leads to a validation error, the file is lost on the next correct submission and it's working correctly. Files are attached to the webform result after anonymous user corrects validation (email field) and re-submits.
Our enviro D7.43 and Webform 7.x-4.10

To summarize:
- the patch for this current issue (2675170) fixes the upload problem for anonymous users
- the patch at 2678822 #87 solves that issue for us (will make an entry on that issue)

slipperysky’s picture

#93 - Also tested a clean install of Drupal 7.43, Webform 7.x3.24, applied the patches to both of them and didn't see any of the problems.

nicrodgers’s picture

@srutland I've just created a separate issue for the 'fast clicking' bug described in #30 and #89 here: https://www.drupal.org/node/2681011 as (like you say) it's unrelated to the current issue.

smbenoit’s picture

I'm a total Drupal newb, as well as a newb to development/coding. I'm sure this a ridiculously dumb question,
How do I install the provided patch to Webform?
I see the code, but what do I do with it? Does it go in the components folder of Webform? What should the file extension of the text document be called? Do I paste it into the "file.inc" document in the components folder?

I'm about to delete Drupal off of my server, install 7.42, and build the entire website over again from scratch all due to this one problem. I'm hoping someone can give a solution before I do that.

Thank you.

damienmckenna’s picture

nicxvan’s picture

I am using the patch mentioned in #100 as well as #24 here as I am on 3.x

I found the upload issue to still be intermittent, it seems to be related to required fields. If all required fields are filled out before uploading it works. I ended up just marking all fields as optional temporarily for this particular customer and it seems to be working.

squarecandy’s picture

#49 works for me.
I also applied #22 from here: https://www.drupal.org/node/2678822

Is that what I'm supposed to do? Just the patch from this thread seemed to solve it. A bit confusing with all the co-mingled related-but-different issues.

Thanks!

dcamburn’s picture

Do we have an official answer on how this is going to be fixed? Whether Drupal Core will be updated to fix it, or whether Webform will be updated to fix it?

damienmckenna’s picture

@dcamburn: We're waiting for a core committer to respond to the issue :-\

gaurav_manerkar’s picture

Please apply this patch works for me

gaurav_manerkar’s picture

Assigned: Unassigned » gaurav_manerkar
Status: Needs work » Needs review
StatusFileSize
new3.29 KB

My patch works fine, tested by me

Status: Needs review » Needs work

The last submitted patch, 106: ubercart-expiring-roles-not-deleted-2589043-18.patch, failed testing.

damienmckenna’s picture

Status: Needs work » Postponed

Marking this 'postponed' until the core issue is resolved.

bukhnerav’s picture

#105 works for me. Thanks!

jweowu’s picture

DamienMcKenna: webform-file-upload-for-anonymous-user-2675170-2.patch contains a syntax error.

PHP Parse error: syntax error, unexpected '}' in sites/all/modules/contrib/webform/webform.module on line 2727

jweowu’s picture

Indeed, I've noted that #22 from that issue resolves the problem. (Thanks!) It was a bit confusing as #108 in this issue post-dates that, and so my initial impression had been that both patches were necessary to resolve the issue for webform components.

David_Rothstein’s picture

Status: Postponed » Needs work

I committed #2678822: Drupal 7.43 / 8.0.4 regression: When an anonymous user submits a form with an un-uploaded file that leads to a validation error, the file is lost on the next correct submission to Drupal 7, so reopening this. Not sure what the right next step is here now - get the tests committed via this issue, and move the #default_value improvements (at least the ones not labeled "hacks") to a separate issue so they can be evaluated on their own merits, even if no longer needed to fix this bug?

David_Rothstein’s picture

Regarding #67 from @cjallenD1:

I get the following now when uploading multiple files with the patch in #63

An unrecoverable error occurred. The uploaded file likely exceeded the maximum file size (20 MB) that this server supports.

The file in question is well below 20mb (22kb).

That actually sounds a lot like a different (and older) Drupal core bug: #2392117: Forms with multiple file form elements produce upload errors with the Drupal page cache enabled. Perhaps that's the root cause (although a different solution was reported in #72)?

The last submitted patch, 91: webform-n2675170-90.patch, failed testing.

torgospizza’s picture

I just wanted to say that applying the patch from #2678822-22: Drupal 7.43 / 8.0.4 regression: When an anonymous user submits a form with an un-uploaded file that leads to a validation error, the file is lost on the next correct submission also seems to have solved the issue for me. Before, I could consistently get the error to appear by always doing the following steps in Webform:

1. Browse and choose a file to upload to a File field;
2. Click "Upload" and let the file upload.
3. Click "Submit" to submit the form.

9 Times out of 10, Upload -> Submit would fail in a "File field is required" error, but now with that patch applied, the error no longer occurs.

My hunch is that this means this issue and its patch is no longer needed, but I could be wrong.

Thanks for the fix!

glassfrog’s picture

Any word on when this fix will be released in core/webform?

I'm running Drupal 7.43 with webform 7.x-4.12 and I cannot figure out what's happening with all the comments on 7.x/6.x.

The last submitted patch, 49: webform-n2675170-49.patch, failed testing.

The last submitted patch, 91: webform-n2675170-90.patch, failed testing.

liam morland’s picture

The fix has been committed to core. If you run revision 6b6bc1b, you will get the fix. That is just two commits past 7.43.

torgospizza’s picture

liam morland’s picture

The patch from #22 is what got committed to D7 in 6b6bc1b. If you are deploying with Git, just checkout 6b6bc1b and you're done.

dsoini’s picture

I have Drupal 7.43 and webform 7.x-3.24 and I have the problem with file upload fields as mentioned. Additionally, I have the problem that if someone submits the form with missing required fields there is no error message and the form does not submit. It just returns to the form.

If I install the patch (2675170-24.patch), a form submission with missing fields (of any kind) still fails to show an error message. It does fix the file upload issue, but it doesn't solve the issue of no error messages.

Anonymous’s picture

Hi all we just applied the patch. I have multiple file uploads on the same form.
Files are uploaded but do not show up on the results page.

criscom’s picture

We had the same problem as in #19. We solved the problem, patching Drupal core by applying 6b6bc1b as mentioned in #22

priyapr’s picture

Hi crisom,

Could you please confirm whether it solved for webform 7.x-3.24 or webform 7.x-4.12. I applied the patch #22 on Drupal core 7.43 and webform 7.x-3.24, but still it is not get solved the issue reported in #19.

Thanks

priyapr’s picture

Webforms with Mollom or old version google recaptcha (manually enter number or text) or any other modules that provide additional form validation for anonymous users cause the same problem reported in #19 for webform 7.x-3.24 and webform 7.x-4.12. In the above mentioned cases, the page reloading with validation error and uploaded file lost that cause the issue.

Thanks

labboy0276’s picture

Just to ease the confusion, people are mentioning #22, but it is in this issue: https://www.drupal.org/node/2678822#comment-10926613 not this issue.

lu_smithcon’s picture

I'm using Drupal 7.43 and Webform 4.12 and the webform file fields are private. Private Files Download Permissions handles the permissions. I applied drupal-n2678822-22.patch two days ago. I cleared the cache and tested the webform as an anonymous user and it worked as it should. Today the file upload isn't working properly once again. The files are uploaded to the server but are not visible in the webform results (again). In the files_managed table in the db the status of the uploaded files are 0 (temporary) whereas the file from my test submission 2 days ago is 1 (permanent). I have not changed anything since I applied drupal-n2678822-22.patch. Anyone have a clue what is wrong?

erier’s picture

#105 also worked successfully for me. Patch applied to Webform 7.x-4.12

criscom’s picture

priyapr, sorry just saw your comment: we fixed webform 7.x-4.12 with the patch.

drupalevangelist’s picture

#2 Worked for me. Thanks David

ludo.r’s picture

I also have a PHP syntax error when applying patch #105 (https://www.drupal.org/node/2675170#comment-11004815) to Webform 4.12

jweowu’s picture

damien___b’s picture

We have multiple mail systems.
Attachment through mimemail mail system is working now thanks to #2.
Thanks David.

knalstaaf’s picture

I'm using two upload fields and the patch of #49 isn't doing it for me.

Provided a workaround by hiding the file submit buttons of the upload fields with CSS, so the user can't actively use them. As long as none of the file submit buttons are being used, there doesn't seem to a problem.

criscom’s picture

It seems drupal-7.44 has re-introduced the error/bug again.

criscom’s picture

Looks like the commit in #2 didn't make it fully into file.module at all. (https://www.evernote.com/l/ARCteKjTztVN17bl3vLmQ9qpgP4iZoB2vzE)

Below part is missing in the 7.44 version of file.module

diff --git a/modules/file/file.module b/modules/file/file.module
index 9e091af..bf7b07d 100644
--- a/modules/file/file.module
+++ b/modules/file/file.module
@@ -457,6 +457,17 @@ function file_managed_file_process($element, &$form_state, $form) {
       '#markup' => theme('file_link', array('file' => $element['#file'])) . ' ',
       '#weight' => -10,
     );
+    // Anonymous users who have uploaded a temporary file need a
+    // non-session-based token added so file_managed_file_value() can check
+    // that they have permission to use this file on subsequent submissions of
+    // the same form (for example, after an Ajax upload or form validation
+    // error).
+    if (!$GLOBALS['user']->uid && $element['#file']->status != FILE_STATUS_PERMANENT) {
+      $element['fid_token'] = array(
+        '#type' => 'hidden',
+        '#value' => drupal_hmac_base64('file-' . $fid, drupal_get_private_key() . drupal_get_hash_salt()),
+      );
+    }
 // Add the extension list to the page as JavaScript settings.
@@ -533,13 +544,24 @@ function file_managed_file_value(&$element, $input = FALSE, $form_state = NULL)
           $force_default = TRUE;
         }
         // Temporary files that belong to other users should never be allowed.
-        // Since file ownership can't be determined for anonymous users, they
-        // are not allowed to reuse temporary files at all.
-        elseif ($file->status != FILE_STATUS_PERMANENT && (!$GLOBALS['user']->uid || $file->uid != $GLOBALS['user']->uid)) {
-          $force_default = TRUE;
+        elseif ($file->status != FILE_STATUS_PERMANENT) {
+          if ($GLOBALS['user']->uid && $file->uid != $GLOBALS['user']->uid) {
+            $force_default = TRUE;
+          }
+          // Since file ownership can't be determined for anonymous users, they
+          // are not allowed to reuse temporary files at all. But they do need
+          // to be able to reuse their own files from earlier submissions of
+          // the same form, so to allow that, check for the token added by
+          // file_managed_file_process().
+          elseif (!$GLOBALS['user']->uid) {
+            $token = drupal_array_get_nested_value($form_state['input'], array_merge($element['#parents'], array('fid_token')));
+            if ($token !== drupal_hmac_base64('file-' . $file->fid, drupal_get_private_key() . drupal_get_hash_salt())) {
+              $force_default = TRUE;
+            }
+          }
         }
         // If all checks pass, allow the file to be changed.
-        else {
+        if (!$force_default) {
           $fid = $file->fid;
         }
       }
scrypter’s picture

Thanks criscom for confirming this. I was transported back to when 7.43 was released and had to revert. What needs to happen to get this patch into core so that when 7.45 comes out we don't have all this drama again?

damienmckenna’s picture

Just to be clear, the fix is not in 7.44 because that only contained security fixes, no other changes were included. The fix is currently slated for the forthcoming 7.50 release, per David_Rothstein's comment in #62 of the other issue.

hmartens’s picture

I can confirm that with Drupal 7.44 it still didn't work. I had to use Patch #2 and then it worked. Thanks guys for the awesome work!

I appreciate all your hard work!

danica101’s picture

worked for me.
thanks

sic’s picture

The 3 branch patch still does not work properly.

Steps to reproduce:

Two fields Foo and Bar, both required.

Browse/select a file for Foo, submit the form. Errors appears "Bar" is required. Upload a file for Bar, submit the form. Error: Foo is required.

stefan.r’s picture

Status: Needs work » Closed (won't fix)

It seems drupal-7.44 has re-introduced the error/bug again.

That's incorrect as explained in #140.

Shouldn't this issue be a won't fix, since it will be fixed in core as of the upcoming 7.50 release (planned for next month)?

In the mean time, people should use the core patch (https://www.drupal.org/files/issues/drupal-n2678822-22.patch) rather than any patch in this issue

webservant316’s picture

Is this patch still needed with drupal 7.50? Which patch above?
I see the core patch is included with drupal 7.50. Sorry about that.

The last submitted patch, 49: webform-n2675170-49.patch, failed testing.

The last submitted patch, 63: webform-n2675170-63.patch, failed testing.

crutch’s picture

Just completed upgrades and testing using core 7.50. Webform 7.x-4.13 and webform 7.x-4.5 were both tested with anonymous user. They both work fine and able to access the documents in results tables. Captcha module has an issue, re: https://www.drupal.org/node/918856#comment-11526361

damienmckenna’s picture

Status: Closed (won't fix) » Closed (duplicate)

Changing to "duplicate" rather than "won't fix" as the primary issue was in core, not Webform.

Other problems should be handled via separate issues.

roychan’s picture

Here is how we fix the problem without patching. To accept anonymous uploads where Drupal doesn't if file was uploaded in the same session.

Instead of patching the webform module, we inject an extra process to the managed file type as a temp fix, to make sure both $form_state['input'] & $form_state['value'] comes with the same fid as we can also found in the session.

/**
 * Implements hook_element_info_alter().
 */
function example_fixes_element_info_alter(&$type) {
  if (isset($type['managed_file'])) {
    $type['managed_file'] += ['#process' => []];
    array_unshift($type['managed_file']['#process'], 'example_managed_file_webform_process');
  }
}

function example_managed_file_webform_process($element, &$form_state, $complete_form) {
  // Accept anonymous uploads where Drupal doesn't if file was uploaded in the same session.
	if (!empty($form_state['process_input']) && user_is_anonymous()) {
		$input = drupal_array_get_nested_value($form_state['input'], $element['#parents']);

		if (empty($element['#value']['fid']) && !empty($input['fid']) && isset($_SESSION['webform_files'][$input['fid']])) {
			form_set_value($element, $input, $form_state);
			$element['#value']['fid'] = $input['fid'];
		}
	}

	return $element;
}
gaurishankar’s picture

patch #2 works for me (version: 7.x-4.12)
Thanks!

melsi’s picture

Updated patch for 7.x-4.16.

caspervoogt’s picture

Patch #152 worked for me on 7.x-4.17, though it had trouble locating the files to patch (I had to manually specify the file path for each file it wanted to patch).

caspervoogt’s picture

I have just had a recurrence of this issue using 7.x-4.19 (unpatched) and core version 7.63. I then applied patch #152, and that solved it. Maybe this patch should be tested by a few others. I understand core applied some relevant changes in 7.50, but somehow something appears to have changed between 7.50 and 7.63.

damien_bates’s picture

I've just had a recurrence of this issue as well using webform 7.x-4-20 and core version 7.67. Like caspervoogt, I applied patch #152 and the problem was resolved. Had the same issue, the file was using a private file directory and worked when just the submit button was used by an anonymous user. However, once the user used the upload button below the field an error indicating the file was required would continue to be thrown.

liam morland’s picture

Are there steps to reproduce the problem on a clean install? The patch needs to be rolled against the current development version.

spokje’s picture

StatusFileSize
new4.79 KB

Reroll of 7.x-3.x patch in #24 against latest HEAD of the 7.x-3.x branch.

den tweed’s picture

Reroll for 7.x-4.23