Hello,

Since I updated to Drupal 7.39, I get an error whenever I try to add a new menu item with a menu view.

Here's what happens:
1) go to admin/structure/menu/manage/main-menu/add
2) Menu item type: select View
3) Select applicable view.
4) Instead of display list, I get this error:

"The form has become outdated. Copy any unsaved work in the form below and then reload this page."

I'm using Drupal 7.39, Views 7.x-3.11 and Menu Views 7.x-2.2

I've found a hack to fix this. Looking about in related errors, I found this thread for another module: https://www.drupal.org/node/1383918

Adopting that method, I edited the file menu_views.admin.inc. I added a line to the top of function _menu_views_form_alter(array &$element, array &$form, array &$form_state). Top of the function, line 142 for me, add a new line:

unset($form['#token']);

So the top of the function looks like:

function _menu_views_form_alter(array &$element, array &$form, array &$form_state) {
	unset($form['#token']);
  // Still need to render the form, just hide it for those who don't have access.
  $access = user_access('administer menu views');
  // Only need to attach or alter form if user has access.

I attached a screenshot of the error.

Comments

daggar created an issue. See original summary.

patrickvanelk’s picture

I can confirm this issue. Completely disabled caching to rule that out as the cause, but unfortunately it didn't help...

revjtanton’s picture

I think changes to ajax and form api caused this? https://www.drupal.org/SA-CORE-2015-003

NeuQuest’s picture

I can confirm this error also, but fortunately dagger's fix works. Can we get this committed?

millionleaves’s picture

Ditto for me.

Error confirmed, daggar's fix works - thanks.

Module doesn't work (for me) without this fix.

jwineichen’s picture

Same as #5 for me.

Izoman’s picture

Please fix this. This is major bug/to be fixed.

tigin öztürk’s picture

Thanks dagger.

mautumn’s picture

Am I blind or stupid - I can't see it anywhere - has anyone very kindly produced a patch for this?

If not...

menu_views-ajax-error-2557327.patch

diff --git a/modules/contrib/menu_views/menu_views.admin.inc b/modules/contrib/menu_views/menu_views.admin.inc
index f5b05ec..6734813 100644
--- a/modules/contrib/menu_views/menu_views.admin.inc
+++ b/modules/contrib/menu_views/menu_views.admin.inc
@@ -139,6 +139,10 @@ function _menu_views_form_tokens_ui(array &$element) {
  *   The complete form state array passed by reference.
  */
 function _menu_views_form_alter(array &$element, array &$form, array &$form_state) {
+  // to fix: "The form has become outdated. Copy any unsaved work in the form below and then reload this page." 
+  // see https://www.drupal.org/node/2557327#comment-10357003
+  unset($form['#token']);
+
   // Still need to render the form, just hide it for those who don't have access.
   $access = user_access('administer menu views');
   // Only need to attach or alter form if user has access.
martynov’s picture

Thank you, daggar!

karlshea’s picture

Status: Active » Reviewed & tested by the community

Patch worked for me.

brankolo’s picture

Patch works

same as #5 for me

Thanks

Robin_K’s picture

Patch works for me too, thanks!

skyredwang’s picture

Here is the patch for #1

It seems that the maintainer hasn't been active. If no one else has access to this project Git, I can try to contact the author to get this patch committed and make a new release.

skyredwang’s picture

StatusFileSize
new741 bytes

The original file wasn't saved by Drupal standard, that's why there is so many trivial edits in the previous patch. I redid the patch using nano.

markhalliwell’s picture

Version: 7.x-2.2 » 7.x-2.x-dev
Status: Reviewed & tested by the community » Needs work

I've found a hack to fix this.

A "hack" is a hack. This patch unsets the form #token, where the primary purpose is to help prevent forged form submissions (e.g. this would introduce a security issue).

I'm actually quite surprised that this reached RTBC status so quickly. This issue is most certainly not committable as is.

It seems that the maintainer hasn't been active.

I've been monitoring this issue (I get email notifications for this project), but have yet to reply hoping that someone would see the same thing and say something before I would have to get involved. This project's maintenance and development statuses have been marked appropriately for a while now, however it's definitely not high up on my todo list at the moment.

robertstaddon’s picture

@markcarver, the Drupal documentation for drupal_prepare_form (https://api.drupal.org/api/drupal/includes%21form.inc/function/drupal_pr...) actually unsets the #token value itself for constructors where when cross site request forgery is irrelevant to the form:

// Form constructors may explicitly set #token to FALSE when cross site
// request forgery is irrelevant to the form, such as search forms.
if (isset($form['#token']) && $form['#token'] === FALSE) {
unset($form['#token']);
}

I think that the way that the "Menu Views" module is interfacing with the form in this scenario may be one of these types of situations. I would suggest that the fix from @daggar isn't really a hack, then, but rather a valid method for dealing with a form where cross site request forgery is irrelevant.

markhalliwell’s picture

My stance on this issue remains the same. Here are the reasons why:

Form constructors may explicitly set #token to FALSE

The Menu Views module doesn't create the form, it merely alters it. It isn't solely responsible for validating the form, nor do I wish it to be.

when cross site request forgery is irrelevant to the form, such as search forms.

The use case that is described here is when the form is used for simple DB retrieval, without the risk of overwriting (saving) something to the DB. Editing a menu item is not "irrelevant" and doesn't fall under this use case.

Also, FWIW, I have already asked a couple [current & former] members of the security team (my co-workers at Tag1) and they agreed that this is not the recommended approach for this module. I don't have the time myself to debug this, but I do know that the current approach is indeed a hack. Is it a hack I'm willing to commit, not entirely sure yet. I will, however, be highly resistant to this until someone has at least considered a different approach in trying to fix this and given very clear reasons why this current approach is the "only way".

skyredwang’s picture

I will try to follow @markcarver advice to seek a proper fix, if I can manage to find the time.

But my 2 cents: Given this module is used by most Drupal admin, normal users wouldn't have access to this form. If the site cannot trust such user, then the site has a bigger problem.

markhalliwell’s picture

Given this module is used by most Drupal admin, normal users wouldn't have access to this form. If the site cannot trust such user, then the site has a bigger problem.

That's not true. The very nature of XSS vulnerabilities are about exploiting said "trusted user" whom has elevated permissions. One of the types of XSS attacks is about spoofing form submissions, often without said user knowing about it, simply because said user is "logged in". If you take out the form token, it prevents the form from validating that the logged in user actually visited the page, thus generating the token, before the form is submitted.

P.S. I'm not saying that this is necessarily a "critical" security issue, but it is a security issue nonetheless.

Ambroise A’s picture

For the moment, the module doesn't work without Daggar's patch. Anything new on the second approach?

Thanks

thomas1977’s picture

Would like an update as well... great module - needs to be fixed. And if the patch is working - should be a no-brainer?

(on a sidenote: I've been building websites with Drupal for 6 years now, and I always run into issues like this - some essential module that needs some critical update that just seems like forever to be released... or never even gets that far. So, as much as I love Drupal as a platform and its active and sharing open source society, I always end up pulling some hairs off along the way... and right now, I really need this module to work :-) )

markhalliwell’s picture

The attached patches are not acceptable, as I have already stated in #16. I do not have the time to work on or maintain this project anymore, as I have also already stated in #18.

skyredwang’s picture

For the record, I appreciate more that @markcarver reviewed the patch, and decided the quality wasn't good enough to release than otherwise lowering the standard and focusing on short-run gain like described in #22

thomas1977’s picture

I absolutely agree that securing the module quality is top priority. However, the problem here is far more basic in that module hasn't worked for some time now, and for people like me - that lack the necessary coding skills - there doesn't seem to be any other way than to use a patch that brings back core functionality. So, I use the patch - and for the time being I can live with the fact that a better solution is not commited to the official version. But what about in one, two, three months or a year? Maybe the patch is going to need a new patch due to changes in other modules of drupal core itself? Anyway, it's a great module (when working), and I hope someone else with the coding skills will pick up the challenge and maintain it :-) All the best. Appreciate all your work.

Buckling_Spring’s picture

StatusFileSize
new812 bytes

I stumbled upon this issue only recently and was able to make the Menu Views form work again in my current project by applying the fix in the attached patch. Tested with 7.41 - please feel free to try it out and let me know if it works for you.

markhalliwell’s picture

#26 is more in line with what I had imagined. However, it's manually constructing this information. Surely there has to be a more appropriate solution here. As I stated above, this module does not create the form, it merely alters it. Shouldn't a token already exist in the form?

Buckling_Spring’s picture

Shouldn't a token already exist in the form?

Yes, there is a token. However, it gets removed from the form when the AJAX request to display the fields introduced by Menu Views is issued. I will take a closer look on what is causing this behaviour and modify the patch to preserve the original token, instead of generating a new one.

Since I don’t have access to my testing environment right now, it will take some time though.

sgdev’s picture

Status: Needs work » Needs review
StatusFileSize
new1.3 KB

Yes, there is a token. However, it gets removed from the form when the AJAX request to display the fields introduced by Menu Views is issued.

The token is not removed. It has been set to #access = FALSE, when it should not. The problem is the code in _menu_views_form_alter that loops through all child elements, and sets #access to TRUE if the element should be visible.

When the form is initially built, form_build_id, form_token, and form_id are set to #access = TRUE. However, the form_alter code does not skip these children, and sets them to FALSE. By doing so, they are not accessible to the updated form after the ajax processing is complete.

Please review the attached patch. Thanks.

Status: Needs review » Needs work

The last submitted patch, 29: menu_views-ajax_error-2557327-29.patch, failed testing.

sgdev’s picture

Status: Needs work » Needs review
StatusFileSize
new1.29 KB

Sorry, I had looked at an old version of the module when creating my patch, and should not have included a reference to 'xmlsitemap'. Please see the attached version instead. Thanks.

Status: Needs review » Needs work

The last submitted patch, 31: menu_views-ajax_error-2557327-30.patch, failed testing.

sgdev’s picture

Not sure why this is failing. In the console output, it's returning the following messages:

14:33:00 ERROR: No valid tests were specified.
14:33:01 PHP Notice:  Undefined variable: classes in /opt/drupalci_testbot/src/DrupalCI/Plugin/BuildSteps/publish/JunitXMLFormat.php on line 144
14:33:01 PHP Warning:  Invalid argument supplied for foreach() in /opt/drupalci_testbot/src/DrupalCI/Plugin/BuildSteps/publish/JunitXMLFormat.php on line 169
14:33:01 ERROR: Step ?Publish JUnit test result report? failed: None of the test reports contained any result
14:33:01 Finished: FAILURE
markhalliwell’s picture

Ah ha! That makes a lot of sense. I completely forgot it was doing this.

Regarding the "test", this module doesn't actually have any tests and it's likely just a Drupal CI problem (they've been working on it quite extensively).

I'm willing to commit #31 (with a few adjustments) to dev.

Once it's in the dev release and a few people from this issue have tested it and marked it RTBC, I'll make another release.

  • markcarver committed 1c5666f on authored by ron_s
    Issue #2557327 by skyredwang, ron_s, Buckling_Spring: Ajax error on Menu...
markhalliwell’s picture

Status: Needs work » Needs review

  • markcarver committed 9df754e on
    Issue #2557327 by markcarver, skyredwang, ron_s, Buckling_Spring: Ajax...
markhalliwell’s picture

In retrospect, I also added the "token" element type to the ignored types. I suppose this was always an issue, but mitigated by the fact that AJAX requests didn't properly check for a form token before 7.39.

markhalliwell’s picture

Status: Needs review » Fixed

Actually, should probably just mark this as "Fixed" since I made some commits.

Will only make a release once people have commented that it works.

langelhc’s picture

When I installed 7.x-2.2 I got the same error, "The form has become outdated. Copy any unsaved work in the form below and then reload this page."

Then I installed 7.x-2.x-dev version and works great.

Thanks people.

markhalliwell’s picture

Also, I'd like for some willing to look at these following issues. My hope is that I can get them in/closed before a release is made too so I don't have to come back through here later:
#2422993: Allow contextual filter arguments to be used in view title
#2180121: Doesn't respect view sort and/or is cached somehow and can't be reset
#2411009: Description field conflict with Menu Attributes module

langelhc’s picture

StatusFileSize
new56.71 KB

Just in case you are using bootstrap, I get this particular error:

When I go to edit the menu-views item and select another display, after the ajax has loaded it looks like JS is broken because fieldsets are not working (menu_views-error-on-edit.png)

I get this error in apache log:

Warning: Unknown: Input variables exceeded 1000. To increase the limit change max_input_vars in php.ini. in Unknown on line 0, referer: admin/structure/menu/item/725/edit

If I try to save the form I get this error in apache log:

PHP Fatal error: Call to undefined function _bootstrap_process_element() in /home/angel/pry/freelos/porvenir/docroot/includes/form.inc on line 1870, referer: /admin/structure/menu/item/725/edit

My solution:
I get a solution increasing max_input_vars in php.ini

markhalliwell’s picture

That has absolutely nothing to do with this issue (or the ones I linked, related by wanting to create release soon)...

rlujano’s picture

I installed and tested this module in version 7.x-2.x-dev in a clean drupal environment, and don't have any problems.

markhalliwell’s picture

Ok, the issue queue for menu_views has been pruned and quite a few issues "fixed" and one new feature.

I'll make a release tomorrow, maybe Tuesday, to can people a chance to check everything out.

Status: Fixed » Closed (fixed)

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