Using Field Collection I uploaded a file and received these warnings:
Warning: Invalid argument supplied for foreach() i file_field_widget_submit() (rad 760 av /home/storlede/public_html/modules/file/file.field.inc).
Warning: array_values() [function.array-values]: The argument should be an array i file_field_widget_submit() (rad 767 av /home/storlede/public_html/modules/file/file.field.inc).
Turns out that $submitted_values isn't checked for type before used in foreach and array_values functions. Patch is coming shortly.
Comments
Comment #1
ordermind CreditAttribution: ordermind commentedHere's the patch! I'm still rather new to making patches so I hope it'll pass testing.
Comment #3
BerdirYou need to use two spaces instead of tabs.
Comment #4
ordermind CreditAttribution: ordermind commentedThanks! Trying again...
Comment #5
ordermind CreditAttribution: ordermind commentedForgot to change the issues status :)
Comment #7
njcheng CreditAttribution: njcheng commentedJust want to reference #1329856: D7.9: Invalid argument supplied for foreach() in file_field_widget_submit - line 753 that seems related to exactly this issue.
Comment #8
tim.plunkettRerolled the patch.
Note, all this does is wrap it in
if (is_array($submitted_values)) {
, the rest is indenting.This will need to be moved to D8, but posting this first so that I can point everyone from #1329856: D7.9: Invalid argument supplied for foreach() in file_field_widget_submit - line 753 at something.
Comment #10
sovarn CreditAttribution: sovarn commentedComment #11
dsnoeck CreditAttribution: dsnoeck commentedThis patch fix the issue for me on D 7.12, thanks.
Comment #12
xturgorex CreditAttribution: xturgorex commentedGreat! Worked for me on D7.12 as well. Thanks!
Comment #13
enzipher CreditAttribution: enzipher commentedConfirming that the patch applied cleanly to 7.12. Thanks!
Comment #14
GaëlGMight be related to #1387258: Relation Add_block error on file upload, but the last patch does not fix it.
Comment #15
tvilms CreditAttribution: tvilms commentedI tried the patch from #8. The error went away, however, after hitting "upload" (before even saving the node) the file doesn't show as attached to the node. I do see the file ends up in the appropriate directory and it's listed in the {file_managed} table. Has anyone seen this behavior, too? For me - it means the patch doesn't work.
Comment #16
Niklas Fiekas CreditAttribution: Niklas Fiekas commentedThis would have to go into D8 first. Settings needs work for a reroll. Tagging novice to reroll #8. Thanks @ordermind and @tim.plunkett!
(For the chance of it: Is this reproducable with just core? If yes excact steps to reproduce would be awesome.)
Comment #17
tim.plunkettI don't think there was a way to reproduce with just core, but we'll need to come up with something for the tests.
Comment #18
tvilms CreditAttribution: tvilms commentedFollow up on my comment #15. I had the file field as a dependent field to another, using Conditional Fields. When I removed the dependency, the error went away. Hope that helps anyone else looking at this issue.
Comment #19
daniela basualdo CreditAttribution: daniela basualdo commentedI had the same Problems:
So then I found out that the problem was not in core file.field.inc, it was the problem in another Contributed Module.
In my case, the module used field_attach_form() to add some fields to an menu item. Instead of passing the whole $form array, it was passed a sub-element like $form['power_menu']['fields'].
The error dissapeard after I changed it to $form. I also was able to add another image and safe it properly.
Comment #20
daniela basualdo CreditAttribution: daniela basualdo commentedComment #21
j-phat CreditAttribution: j-phat commented#8: drupal-1468686-8.patch queued for re-testing.
Comment #23
dsdeiz CreditAttribution: dsdeiz commentedPatch attached for D8.
Comment #24
tim.plunkettCNW for tests.
Comment #25
tim.plunkettRemoving the novice tag.
Comment #26
paranojik CreditAttribution: paranojik commentedI found a way to reproduce this. I turns out, you just have to move the file field in hook_form_alter into a container and upload a file. Working on a test....
Comment #27
paranojik CreditAttribution: paranojik commentedAnd here's the patch with the test implemented.
Comment #28
star-szrPossibly related to #1545584: Problem when using Image field with field collection, which already has a fix in, but no tests.
Comment #29
andrewbelcher CreditAttribution: andrewbelcher commentedUnfortunately I'm not sure it's that simple. This is actually using the wrong parent key.
$form_state['values']
uses the structure given by#parents
, not#array_parents
which is for the structure of the$form
array.This is also why it never happens in core, as all the instances of this in core have
#array_parents
and#parents
matching.What is actually needed to fix this is to use the
#parents
of the$element
. Once that is done, I don't know if we need to check that it is an array.I've tested the D7 patch, the D8 one is identical except the folder structure of Drupal has changed for D8.
Comment #30
paranojik CreditAttribution: paranojik commentedTest should still apply... D8 only.
Comment #31
quicksketchConfirmed. As the person who wrote the incriminating line I claim complete ignorance on behalf of my former self. #array_parents was a new (undocumented) property at the time. Now it's more clear to me that #parents means "location in form_state/$_POST" while #array_parents means "true location in $form".
There's a typo in @paranojik's latest patch:
he == the
I'm not sure about including this test at all. Seems really convoluted to me and I'd rather see #29 committed as-is. It's extremely unlikely that this *exact line* will changed back to using #array_parents, which is the extent of the test.
Comment #32
YesCT CreditAttribution: YesCT commented#29: drupal_core-wrong_parent_key_in_field_field_widget_submit-1468686-29-d8.patch queued for re-testing.
Comment #33
andrewbelcher CreditAttribution: andrewbelcher commentedI've just looked at the current state of
file_field_widget_submit()
and the code has completely changed since I wrote that patch (I think mainly for updates to Form API and some OO bits). Not sure if the changes have already fixed the problem already, I will try and test it and if not, post up a new patch.Comment #34
gappleThe posted patch for D7 didn't work with the modified array_slice parameters. Keeping the original '-2' and just changing '#array_parents' to '#parents' seems to resolve the issue.
Comment #35
andrewbelcher CreditAttribution: andrewbelcher commented@gapple - the reason it didn't work I suspect is that you were still getting it from
$button
rather than$element
as my patch was... Not sure whether$button
or$element
is the better one to use...Comment #36
rlmumford@gapple is exactly right! This is all the fix that was required (for drupal 7) am going to look into the drupal 8 equivalent on my lunch break. It's such a shame that we have to wait for the D8 fix when the Drupal 7 fix is *right there*
Comment #37
rlmumfordHere's a fix for the latest code in drupal 8.
Comment #38
Georgique CreditAttribution: Georgique commentedPatch for D7 (do not test while we are waiting for patch for D8)
Comment #39
rlmumfordPatch in #38 is identical to the patch in #34. Thanks for posting though.
Comment #40
Georgique CreditAttribution: Georgique commented@rlmumford Ah, missed #34, sorry.
Comment #41
jibran#37: 1468686-37.patch queued for re-testing.
Comment #43
xps CreditAttribution: xps commentedThis issue for 8.x was fixed in #625958: Support Uploading Multiple Files for HTML5 Browsers via #multiple attribute. Here is the patch for 7.x.
Comment #44
xps CreditAttribution: xps commented43: 1468686.patch queued for re-testing.
Comment #45
Georgique CreditAttribution: Georgique commented@xps Since your patch is identical to #34 and #38, I can say it works for me for a several months.
Comment #46
Georgique CreditAttribution: Georgique commentedComment #48
David_Rothstein CreditAttribution: David_Rothstein commented43: 1468686.patch queued for re-testing.
Comment #49
David_Rothstein CreditAttribution: David_Rothstein commented(We'll see if the test failure was real....)
Comment #50
kamila12 CreditAttribution: kamila12 commented43: 1468686.patch queued for re-testing.
Comment #51
David_Rothstein CreditAttribution: David_Rothstein commentedSome kind of tests would be nice here because it theoretically indicates a gap in test coverage, but if it's convoluted (and if it was never tested when fixed in Drupal 8) I guess it's OK... I do agree that it's not too likely anyone would modify this code again in a way that breaks this.
So, committed to 7.x - thanks!
Comment #53
quicksketchThanks @David_Rothstein, great to see this one fixed!