Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
This issue occurs on both the current 2.0 release and the previous 1.x release. To reproduce:
- Create a webform with a file upload field.
- Protect the form with mollom -- captcha/analysis doesn't seem to matter.
- Submit the form with a user that cannot bypass mollom
.. and you'll get a WSOD. The appears to be that when mollom cleans a clone of the form state via form_state_values_clean, the value of the file field is already set to the FID, and is not an array as that function expects. Here's a sample stack trace:
[Wed Apr 04 11:13:31 2012] [error] [client 127.0.0.1] PHP Fatal error: Cannot unset string offsets in [local fs path]/includes/form.inc on line 2146, referer: http://dev.localhost/node/51
[Wed Apr 04 11:13:31 2012] [error] [client 127.0.0.1] PHP Stack trace:, referer: http://dev.localhost/node/51
[Wed Apr 04 11:13:31 2012] [error] [client 127.0.0.1] PHP 1. {main}() [local fs path]/index.php:0, referer: http://dev.localhost/node/51
[Wed Apr 04 11:13:31 2012] [error] [client 127.0.0.1] PHP 2. menu_execute_active_handler() [local fs path]/index.php:21, referer: http://dev.localhost/node/51
[Wed Apr 04 11:13:31 2012] [error] [client 127.0.0.1] PHP 3. call_user_func_array() [local fs path]/includes/menu.inc:517, referer: http://dev.localhost/node/51
[Wed Apr 04 11:13:31 2012] [error] [client 127.0.0.1] PHP 4. node_page_view() [local fs path]/includes/menu.inc:0, referer: http://dev.localhost/node/51
[Wed Apr 04 11:13:31 2012] [error] [client 127.0.0.1] PHP 5. node_show() [local fs path]/modules/node/node.module:2605, referer: http://dev.localhost/node/51
[Wed Apr 04 11:13:31 2012] [error] [client 127.0.0.1] PHP 6. node_view_multiple() [local fs path]/modules/node/node.module:1407, referer: http://dev.localhost/node/51
[Wed Apr 04 11:13:31 2012] [error] [client 127.0.0.1] PHP 7. node_view() [local fs path]/modules/node/node.module:2534, referer: http://dev.localhost/node/51
[Wed Apr 04 11:13:31 2012] [error] [client 127.0.0.1] PHP 8. node_build_content() [local fs path]/modules/node/node.module:1285, referer: http://dev.localhost/node/51
[Wed Apr 04 11:13:31 2012] [error] [client 127.0.0.1] PHP 9. module_invoke_all() [local fs path]/modules/node/node.module:1387, referer: http://dev.localhost/node/51
[Wed Apr 04 11:13:31 2012] [error] [client 127.0.0.1] PHP 10. call_user_func_array() [local fs path]/includes/module.inc:819, referer: http://dev.localhost/node/51
[Wed Apr 04 11:13:31 2012] [error] [client 127.0.0.1] PHP 11. webform_node_view() [local fs path]/includes/module.inc:0, referer: http://dev.localhost/node/51
[Wed Apr 04 11:13:31 2012] [error] [client 127.0.0.1] PHP 12. drupal_get_form() [local fs path]/sites/all/modules/webform/webform.module:1498, referer: http://dev.localhost/node/51
[Wed Apr 04 11:13:31 2012] [error] [client 127.0.0.1] PHP 13. drupal_build_form() [local fs path]/includes/form.inc:123, referer: http://dev.localhost/node/51
[Wed Apr 04 11:13:31 2012] [error] [client 127.0.0.1] PHP 14. drupal_process_form() [local fs path]/includes/form.inc:366, referer: http://dev.localhost/node/51
[Wed Apr 04 11:13:31 2012] [error] [client 127.0.0.1] PHP 15. drupal_validate_form() [local fs path]/includes/form.inc:835, referer: http://dev.localhost/node/51
[Wed Apr 04 11:13:31 2012] [error] [client 127.0.0.1] PHP 16. _form_validate() [local fs path]/includes/form.inc:1108, referer: http://dev.localhost/node/51
[Wed Apr 04 11:13:31 2012] [error] [client 127.0.0.1] PHP 17. form_execute_handlers() [local fs path]/includes/form.inc:1373, referer: http://dev.localhost/node/51
[Wed Apr 04 11:13:31 2012] [error] [client 127.0.0.1] PHP 18. mollom_validate_analysis() [local fs path]/includes/form.inc:1432, referer: http://dev.localhost/node/51
[Wed Apr 04 11:13:31 2012] [error] [client 127.0.0.1] PHP 19. mollom_form_get_values() [local fs path]/sites/all/modules/mollom/mollom.module:1567, referer: http://dev.localhost/node/51
[Wed Apr 04 11:13:31 2012] [error] [client 127.0.0.1] PHP 20. form_state_values_clean() [local fs path]/sites/all/modules/mollom/mollom.module:1047, referer: http://dev.localhost/node/51
Comment | File | Size | Author |
---|---|---|---|
#16 | 1515158-form-state_values_clean.patch | 1.66 KB | Crell |
#6 | 1515158.form_state_values_clean.6.patch | 1.26 KB | katbailey |
#4 | mollom-webform-file-upload-WSOD-1515158-4.patch | 3.07 KB | shrop |
Comments
Comment #1
shrop CreditAttribution: shrop commentedI can confirm that this is happening in 7x-1.1. Running PHP 5.3. I am investigating further.
Comment #2
gamelodge CreditAttribution: gamelodge commentedYes, This is happening for me too.
Comment #3
JonMcL CreditAttribution: JonMcL commentedYes, just confirmed that this is happening for me as well. 7.x-1.1 and 7.x-2.0. PHP 5.3 and 5.2.
mollom_form_get_values calls form_state_values_clean and it seems to have trouble on line 2144 (7.x-2.0) trying to unset the button for the file upload.
Comment #4
shrop CreditAttribution: shrop commentedThis patch is a temporary fix to allow webform submissions which include file uploads to work with the mollom module. Per http://drupal.org/node/1515158, Webform sets the fid via ajax and core's form_state_values_clean() cannot unset related fields. This code modifies core's form_state_values_clean() to skip file upload related field submitted via Webforms. The fixed version of the function is _mollom_form_state_values_clean().
This fix allows our forms to submit, while maintaining mollom processing. This doesn't feel like the best solution for this issue, but seems to work so far. Please let me know any improvements or suggestions for this patch.
Comment #5
stevengraff CreditAttribution: stevengraff commentedI tried applying this patch using the patch command and it failed:
I then patched the second hunk manually, and the patch seems to work.
Comment #6
katbailey CreditAttribution: katbailey commentedIt seems to me that the onus is on webform to fix this, since it is webform that is causing the structure of the values array to be different than what form_state_values_clean() would normally expect. The error does not happen with a regular node form with a file upload field that is checked by mollom.
Here's a suggestion of a way around this in webform module's file component. It is not ideal in that it uses a validate hook to remove the upload button from the $form_state['buttons'] array, which is a bit of an abuse of a validate hook, not to mention the fact that I don't know what possible side effects this removal might have. But I don't see where/how else we can get in and prevent the incompatible structure of $form_state (i.e. where the values array no longer corresponds properly to the buttons array) from causing this fatal error.
Comment #7
quicksketchHmm, not sure about this. I'll need to think about it. I'm trying to reduce the amount of 3rd-party integrations in Webform where possible, especially weird work-arounds like this.
Comment #8
deekayen CreditAttribution: deekayen commentedThe patch in #4 was to Mollom specifically, not webform, and it spells mollum instead of mollom.
Re #7, I wouldn't reject #6 for being a third-party integration. The only mention of mollom is in a comment, there's no module_exists() call. If you want it changed, then you should reject it for not being the best solution to the issue.
Comment #9
quicksketchRight exactly. It's a *very* loosely related bit of code that fixes a problem specifically for one module. It's difficult to understand what the code is actually accomplishing, considering it's intended to fix a very specific problem. If we can fix this in a way that doesn't require some kind of data "cleanup" that would be preferable for me.
Comment #10
quicksketchIs there any way we can prevent mollom from skipping validation on the upload of files? Even though it doesn't cause a problem on the node form, mollom doesn't have any reason to be executing its validation in either place.
The core definition of the managed_file element already sets #validate to an empty array(), how's mollom getting its validations in here when the managed_file element is telling everything not to do validations?
http://api.drupal.org/api/drupal/modules%21file%21file.module/function/f...
Comment #11
quicksketchOh, reading the issue some more, this problem doesn't happen on file upload but on submission of the form. I'll need to look into this more. Sorry for the confusion.
Comment #12
shrop CreditAttribution: shrop commented@quicksketch: I had a similar (seemed related) issue between a Webform with a file upload field and http://drupal.org/project/hidden_captcha. I need to get more info on that to be more helpful though.
Comment #13
quicksketchMollom team: After doing some work in the Webform queue recently and thinking about this problem, it struck me: Why doesn't mollom just put their #validate or #submit handler at the beginning of the list instead of the end? Then it won't be affected by Webform (or other modules) modifying their submission data. If I'm still not understanding the issue quite properly I apologize, I still haven't dug into the mollom code to get a full grasp on the problem.
Comment #14
nostromo-1 CreditAttribution: nostromo-1 commentedI get the same problem with the Webform/Mollom/image upload combo, but only with anonymous users. When I am logged in it works fine.
It seems the error occurs at line 2160 my includes/form.inc file (D7.14). The code is
For some reason $values is a string (and holds a number) and PHP (5.2.13) returns true on the isset($values[$last_parent]) check, even when $values is a string. unset is not so forgiving.
A quick fix - which might also catch other problems - is to check whether $values is an array before unset:
This works for now. Perhaps it can be elaborated to include a $values = "" when $values is a string.
NOTE: On the next kernel update the form.inc is likely to be overwritten.
Comment #15
Crell CreditAttribution: Crell commentedThe patch in #6 fixes the fatal, but as a side-effect the uploaded file doesn't appear to actually get saved.
#14: That would make FAPI fatal less, but doesn't hide that the error is elsewhere. Of course, the real problem is that FAPI's design makes errors such as this very easy to introduce and hard to track down. Also, I have no idea what kernel update you're talking about...
Comment #16
Crell CreditAttribution: Crell commentedCorrection. The files ARE saved, they're just not displayed properly on the submission pages. I don't know if that's part of this bug or not, but here's an updated patch (rolled against 3.17, technically, but should be fine) that addresses that problem, too.
I don't know that this is the ideal solution, but it's a solution that Works For Me(tm).
Comment #17
acbramley CreditAttribution: acbramley commentedGetting this exact same error on Webform 7.x-4.0-alpha3
Comment #18
acbramley CreditAttribution: acbramley commentedApplied #16 and it fixed the issue for me.
Comment #19
acbramley CreditAttribution: acbramley commentedThis however introduced a bunch of other bugs, e.g Warning: htmlspecialchars() expects parameter 1 to be string, array given in check_plain() (line 1572 of /var/www/drupal/tpow/releases/20120626-1/includes/bootstrap.inc). on viewing a submission, and when editing a submission various values were not filled out correctly
Comment #20
sunThis is a bug in Drupal core, and I'd recommend to close this as duplicate:
#1090198: managed_file form item in system settings form causes error in form_state_values_clean()
#635046: form_state_values_clean() is polluting form values (followup)
Comment #21
Zaka CreditAttribution: Zaka commentedI applied #16 and it works, but the file cannot be found when you look in the submitted results. I analysed a bit, and I think ignoring the last section of the patch will fix this. I hope this works also for you guys.
function _webform_display_file($component, $value, $format = 'html') {
$fid = isset($value[0]) ? $value[0] : NULL;
+ $fid = isset($value['fid']) ? $value['fid'] : NULL;return array(
'#title' => $component['name'],
'#value' => $fid ? webform_get_file($fid) : NULL,
Comment #22
Krucial CreditAttribution: Krucial commented#16 worked for me.
Comment #23
arthurf CreditAttribution: arthurf commentedI can confirm Zaka's observation in #21.
Comment #24
quicksketchSounds like this patch needs work based on #19 and onwards.
Comment #25
DanChadwick CreditAttribution: DanChadwick commentedThis is an old issue, and I'm not even sure if there is still and issue. Version 7.x-3.x is receiving critical bug fixes only. If there is still an issue with 7.x-4.x, please re-open with new info.