Invalid argument supplied for foreach() in upload_save

markus_petrux - March 12, 2006 - 17:03
Project:Drupal
Version:x.y.z
Component:upload.module
Category:bug report
Priority:critical
Assigned:dopry
Status:closed
Description

After applying the patch committed here:
http://drupal.org/node/42358

When adding/editing a node with no uploads I get the "Invalid argument supplied for foreach()" error related to the following snippet:

<?php
function upload_save($node) {
  foreach (
$node->files as $fid => $file) {
  ...
?>

upload_save() is called from upload_nodeapi(), for op IN ('insert', 'update')

Not sure if this may happen somewhere else, it might be fixed checking if $node->files is an array:

<?php
function upload_save($node) {
  if (
is_array($node->files) {
    foreach (
$node->files as $fid => $file) {
    ...
?>

I think the same problem may happen here:

<?php
function upload_delete_revision($node) {
  foreach (
$node->files as $file) {
?>

Also, I think there is a redundant check in _upload_validate(). The following:

<?php
function _upload_validate(&$node) {
 
// Accumulator for disk space quotas.
 
$filesize = 0;


 
// Check if node->files exists, and if it contains something.
 
if (count($node->files) && is_array($node->files)) {
   
// Update existing files with form data.
   
foreach($node->files as $fid => $file) {
?>

could probably look like:
<?php
function _upload_validate(&$node) {
 
// Accumulator for disk space quotas.
 
$filesize = 0;


 
// Check if node->files is an array.
 
if (is_array($node->files)) {
   
// Update existing files with form data.
   
foreach($node->files as $fid => $file) {
?>

#1

dopry - March 12, 2006 - 18:16
Assigned to:Anonymous» dopry
Status:active» needs review

Here is the sanity checking patch.

I thought upload_load() would cover this for me since it returns and empty array if there a no files, but since I'm leveraging the formAPI these are actually coming from the post. Is there a way to tell the form api to create an empty array of elements if none exist in the form?

This patch should be valid for now.

.darrel.

AttachmentSize
53666.patch 5.99 KB

#2

markus_petrux - March 12, 2006 - 18:26

In the meantime I was also writing a patch. :-/

Mine also removes some empty lines here and there.

AttachmentSize
upload.module.is_array.patch 4.39 KB

#3

chx - March 12, 2006 - 18:46
Status:needs review» reviewed & tested by the community

for markus' patch. Nice work.

#4

dopry - March 12, 2006 - 18:48

+1 on markus's as well...

#5

kkaefer - March 12, 2006 - 20:03

+1, patch works also for me.

#6

UnderDesign - March 13, 2006 - 22:31

Didn't work for me - still getting the same error as reported in node/53816.

Will double-check everything (that all is CVS and patches applied) and post again if error is not recreated.

#7

dopry - March 13, 2006 - 22:35

killes just committed this patch... try updating to head...

You are using simplenews? I'll see if I can replicate..

#8

dopry - March 13, 2006 - 22:46
Status:reviewed & tested by the community» fixed

I just test with cvs head, with simplenews from cvs.

Created a newsletter item without error. using head.

#9

UnderDesign - March 14, 2006 - 00:22

Done it. Was lagging behind on form.inc version.

Everything works now.

Yay.

#10

Anonymous - March 28, 2006 - 00:30
Status:fixed» closed
 
 

Drupal is a registered trademark of Dries Buytaert.