The fix to bug 52484 (Security issue: filter_form needs validation) seems to have broken filter.module on PHP 5. See http://drupal.org/node/48520#comment-101070.

The first argument being passed to filter_form_validate() is a form_id, not the form array itself, which is the second argument. This causes filter_form_validate() to bork out, because it is trying to index the form_id as if it were a form array. I believe this patch fixes the issue and still preserves the security patch, but I am pretty new to Drupal development, so if someone could verify this, that would be great.

Cheers,
Ben

CommentFileSizeAuthor
#14 basicevent.patch1.19 KBbenwei
filter_11.patch373 bytesbenwei

Comments

benwei’s picture

benwei commented
Status: Active » Closed (fixed)

Nevermind, I'm either an idiot or I'm delusional. I did a fresh CVS checkout and the issue seems to be gone. Sorry folks.

dado’s picture

dado commented
Status: Closed (fixed) » Active

Not sure if this is my fault but for the record:
I just upgraded to Drupal 4.7 beta 6 and I get the same error when I try to preview or submit a new node (of my own node type)
I was not getting this error prior to upgrading to beta 6.

I applied the patch in this post and it fixed the error.

Here was the error I was getting
Fatal error: Cannot use string offset as an array in line 835 of ... filter.module

dopry’s picture

dopry commented
Status: Active » Fixed

This is fixed in cvs HEAD. Wait for beta7 if it exists in beta6.

dado’s picture

dado commented
Status: Fixed » Active

I'm still getting this bug. Just downloaded the latest CVS tar.gz file from here
http://drupal.org/files/projects/drupal-cvs.tar.gz
and replaced all core code. Still getting same bug when I try to create content.

Fatal error: Cannot use string offset as an array in C:\workspace\mysite\modules\filter.module on line 835

kkaefer’s picture

kkaefer commented

dado, could you explain exactly what you did to get this error?

dado’s picture

dado commented

I can post story & blog post no problem. However, when I try to create new node from the contrib modules I have tested, I get the error

Fatal error: Cannot use string offset as an array in C:\workspace\mysite\modules\filter.module on line 835
Call Stack
#	Function	Location
1	call_user_func_array ()	C:\workspace\mysite\includes\menu.inc:414
2	call_user_func_array ()	C:\workspace\mysite\includes\form.inc:224

To reproduce error: place latest CVS version of event module's folder in modules directory. enable event & basicevent modules. Go to create content -> event
Fill out minimal info, then click "submit" or "preview". You should get the error. Unless I have done something stupid (likely).

Thanks

chx’s picture

chx commented
Status: Active » Closed (won't fix)

This means that you are running with a pre4.7/bogus filter module. How do I know this? 

  foreach (element_children($form) as $key) {
    if ($form[$key]['#value'] == $form[$key]['#return_value']) {
      return;
    }
  }

so, $form[$key] is a string. However, it's a child. Children are supposed to be arrays in form API. Case closed.

mgifford’s picture

mgifford
he/him
Primary language English
commented
Status: Closed (won't fix) » Active

I'm running into this problem too. My site was running on CVS HEAD, I've since moved it to the last RC. However, I have a parallel site up with the old code. I was running into this problem in php5 when trying to upload images.

So, Just to make sure we're talking apples to apples here, I deleted the filter.module, pulled down a deleted version. Here's what I'm left with:

[mike@team2 modules]$ pwd
/home/dm7_test/modules

[mike@team2 modules]$ cat CVS/Root
:pserver:anonymous:anonymous@cvs.drupal.org:/cvs/drupal

[mike@team2 modules]$ cat CVS/Repository
drupal/modules

[mike@team2 modules]$ less filter.module
<?php
// $Id: filter.module,v 1.113 2006/03/14 15:15:46 unconed Exp $

....

But when I try to upload a image all I get is this in the logs:

[client 64.26.*.*] PHP Fatal error: Cannot use string offset as an array in /home/dm7/modules/filter.module on line 835, referer: http://openconcept.ca/node/add/image

and a blank screen at this path:
node/add/image

So, either I'm missing something or this is an outstanding and critical bug.

Mike

dopry’s picture

dopry commented

@mgifford - a bunch of blank space don't tell me much.

benwei’s picture

benwei commented

Like dado, I am still getting this with the contrib event module, but not with the builtin modules (tested page, blog, book, forum, and story with HEAD).

Based on this, it seems to me this is more accurately classified as a bug in event (and possibly other contrib modules) and not in filter.module. I will see if I can figure out a solution from within event.

benwei’s picture

benwei commented
Component: filter.module » forms system

Doing a search in the drupal base, the only places I could find where custom validation functions like filter_form_validate() were registered for forms were in system.module and filter.module. In both cases these validation functions (password_confirm_validate() and filter_form_validate(), respectively) take only one argument, which is expected to be the form array.

However, when these functions are actually called in _form_validate(), they will have a form_id prepended to their arguments list if such a $form_id is set.

From includes/form.inc, in _form_validate():

$args = array_merge(array($elements), $args);
// for the full form we hand over a copy of $form_values
if (isset($form_id)) {
  $args = array_merge(array($form_id, $GLOBALS['form_values']), $args);
}
if (function_exists($function))  {
  call_user_func_array($function, $args);
}

This is the source of the problem. When password_confirm_validate() and filter_form_validate() are usually called, $form_id is not set, so the first argument actually is the form array, as expected. However when filter_form_validate() is called in event.module, $form_id is set, so it is passed to filter_form_validate() as the first argument instead of the actual form array. It seems to me that _form_validate() should be appending the extra array containing the $form_id to the arguments list at the end, instead of putting it at the beginning. This way, custom validation functions can expect only one argument and have it guaranteed to be the form array instead of the form_id. They can also be written to accept a second argument for the form_id and have it default to null. But maybe there is some piece of history that I'm missing here.

chx’s picture

chx commented
Title: filter.module security update leads to 'Cannot use string offset as an array' error in PHP 5 » filter form validate invalid call
Project: Drupal core » Event
Version: x.y.z » 4.6.x-1.x-dev
Component: forms system » Code

i delegate this to event module but care to tell me, please where event module calls filter_form_validate??

chx’s picture

chx commented
Version: 4.6.x-1.x-dev » 4.7.x-1.x-dev
benwei’s picture

benwei commented
StatusFileSize
new basicevent.patch1.19 KB

OK, I think I've got this figured out. The problem actually lies more specifically in event/contrib/basicevent.module. Near the end of the file, in the function basicevent_form(), the filter format selection form is appended to the event node form like this:

$form = array_merge($form, filter_form($node->format));

filter_form() then sets $form['#validate'] to filter_form_validate(), which is later called from _form_validate() as I discussed above. The problem is that _form_validate() calls filter_form_validate() with a form_id as the first argument, which filter_form_validate() is not expecting. In all of the core modules this doesn't happen because the filter form is it's own separate form within the whole node form. This matters because _form_validate() is recursive and recurses through each child form, but when it calls itself, it does *not* pass a form_id parameter.

The bottom line here is that if you put the filter form as it's own separate form inside the main node form, things work fine, but if you just tack it onto the end, as basicevent.module does, you run into trouble. So, I looked at how page.module does it and basically copied the approach there, and came up with the attached patch, which works pretty nicely for me. Also, I checked to make sure that filter_form_validate() is still being called, and it is.

Let me know what you think.

dado’s picture

dado commented

great work, benwei! This was my problem with my contrib module. The random contrib modules I tested also must have the same code. (basicevent, decisions)

I see this is a bug in drupaldocs, node_example.module code.
http://drupaldocs.org/api/head/file/contributions/docs/developer/example...

return array_merge($form, filter_form($node->format));

I gather that anybody who has copied or will copy node_example.module as a start to their new node will have this bug.

To fix my contrib module, I used benwei's lead and changed the above offending line of code to

$form['format'] = filter_form($node->format);

Not sure if this works for all cases. My node module's form validation is still working

grbitz’s picture

grbitz commented

benwei's patch fixed this for me, fatal error on line 835 of filter.module. Thank you. : )

killes@www.drop.org’s picture

killes@www.drop.org commented
Status: Active » Fixed

applied to cvs.

Anonymous’s picture

(not verified) commented
Status: Fixed » Closed (fixed)