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.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

ordermind’s picture

Here's the patch! I'm still rather new to making patches so I hope it'll pass testing.

Status: Needs review » Needs work

The last submitted patch, file-arraycheck-1468686-1.patch, failed testing.

Berdir’s picture

+++ b/file.field.inc
@@ -757,14 +757,15 @@ function file_field_widget_submit($form, &$form_state) {
+  if(is_array($submitted_values)) {
+	  foreach ($submitted_values as $delta => $submitted_value) {
+	    if (!$submitted_value['fid']) {
+	      unset($submitted_values[$delta]);
+	    }
+	  }
+	  // Re-index deltas after removing empty items.
+  	$submitted_values = array_values($submitted_values);
+	}

You need to use two spaces instead of tabs.

ordermind’s picture

Thanks! Trying again...

ordermind’s picture

Status: Needs work » Needs review
FileSize
1.06 KB

Forgot to change the issues status :)

Status: Needs review » Needs work

The last submitted patch, file-arraycheck-1468686-5.patch, failed testing.

njcheng’s picture

Just 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.

tim.plunkett’s picture

Status: Needs work » Needs review
FileSize
1.11 KB

Rerolled 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.

Status: Needs review » Needs work

The last submitted patch, drupal-1468686-8.patch, failed testing.

sovarn’s picture

Status: Needs work » Needs review
dsnoeck’s picture

This patch fix the issue for me on D 7.12, thanks.

xturgorex’s picture

Great! Worked for me on D7.12 as well. Thanks!

enzipher’s picture

Confirming that the patch applied cleanly to 7.12. Thanks!

GaëlG’s picture

Might be related to #1387258: Relation Add_block error on file upload, but the last patch does not fix it.

tvilms’s picture

I 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.

Niklas Fiekas’s picture

Version: 7.x-dev » 8.x-dev
Status: Needs review » Needs work
Issue tags: +Novice, +Needs backport to D7, +Contributed project blocker

This 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.)

tim.plunkett’s picture

Issue tags: +Needs tests

I don't think there was a way to reproduce with just core, but we'll need to come up with something for the tests.

tvilms’s picture

Follow 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.

daniela basualdo’s picture

Title: Array error in file_field_widget_submit(), file.field.inc » just a hint
Version: 8.x-dev » 7.14
Category: bug » task
Status: Needs work » Active

I had the same Problems:

Warning: Invalid argument supplied for foreach() in file_field_widget_submit() 
Warning: array_values() expects parameter 1 to be array, null given in file_field_widget_submit()

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.

daniela basualdo’s picture

Title: just a hint » Array error in file_field_widget_submit(), file.field.inc
Version: 7.14 » 8.x-dev
Category: task » bug
Status: Active » Needs work
j-phat’s picture

Status: Needs work » Needs review
Issue tags: -Needs tests, -Novice, -Needs backport to D7, -Contributed project blocker

#8: drupal-1468686-8.patch queued for re-testing.

Status: Needs review » Needs work
Issue tags: +Needs tests, +Novice, +Needs backport to D7, +Contributed project blocker

The last submitted patch, drupal-1468686-8.patch, failed testing.

dsdeiz’s picture

Status: Needs work » Needs review
FileSize
1.14 KB

Patch attached for D8.

tim.plunkett’s picture

Status: Needs review » Needs work

CNW for tests.

tim.plunkett’s picture

Issue tags: -Novice

Removing the novice tag.

paranojik’s picture

Assigned: Unassigned » paranojik

I 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....

paranojik’s picture

Assigned: paranojik » Unassigned
Status: Needs work » Needs review
FileSize
4.65 KB

And here's the patch with the test implemented.

star-szr’s picture

Possibly related to #1545584: Problem when using Image field with field collection, which already has a fix in, but no tests.

andrewbelcher’s picture

Title: Array error in file_field_widget_submit(), file.field.inc » file_field_widget_submit(), file.field.inc using the wrong parent key
FileSize
1.2 KB
1.22 KB

Unfortunately 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.

paranojik’s picture

Test should still apply... D8 only.

quicksketch’s picture

Confirmed. 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:

+    // Save the field name to a variable, so that we can alter he right field

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.

YesCT’s picture

andrewbelcher’s picture

Status: Needs review » Needs work

I'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.

gapple’s picture

The 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.

andrewbelcher’s picture

@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...

rlmumford’s picture

@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*

rlmumford’s picture

Status: Needs work » Needs review
FileSize
1.19 KB

Here's a fix for the latest code in drupal 8.

Georgique’s picture

Patch for D7 (do not test while we are waiting for patch for D8)

rlmumford’s picture

Patch in #38 is identical to the patch in #34. Thanks for posting though.

Georgique’s picture

@rlmumford Ah, missed #34, sorry.

jibran’s picture

Status: Needs review » Needs work
Issue tags: +Needs tests, +Needs backport to D7, +Contributed project blocker

The last submitted patch, 1468686-37.patch, failed testing.

xps’s picture

Version: 8.x-dev » 7.x-dev
Status: Needs work » Needs review
Issue tags: -Needs tests, -Needs backport to D7
FileSize
1.2 KB

This 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.

xps’s picture

43: 1468686.patch queued for re-testing.

Georgique’s picture

@xps Since your patch is identical to #34 and #38, I can say it works for me for a several months.

Georgique’s picture

Issue summary: View changes
Status: Needs review » Reviewed & tested by the community

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 43: 1468686.patch, failed testing.

David_Rothstein’s picture

Status: Needs work » Needs review

43: 1468686.patch queued for re-testing.

David_Rothstein’s picture

Status: Needs review » Reviewed & tested by the community

(We'll see if the test failure was real....)

kamila12’s picture

43: 1468686.patch queued for re-testing.

David_Rothstein’s picture

Status: Reviewed & tested by the community » Fixed

Some 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!

  • Commit 106a1e1 on 7.x by David_Rothstein:
    Issue #1468686 by ordermind, paranojik, andrewbelcher, xps, Georgique,...
quicksketch’s picture

Thanks @David_Rothstein, great to see this one fixed!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.