When an anonymous user without the "access profiles" permission tries to view search/user an error message is shown instead of the access denied message.

Fatal error: Cannot unset string offsets in includes/form.inc on line 301

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Jose Reyero’s picture

Confirmed the bug.

it happens also for regular users with access profile permission, when the search type argument is some other word than 'node', 'user'

I.e. for this path: /search/one/two

Jose Reyero’s picture

Status: Active » Needs review
FileSize
1.48 KB

I think the problem was the menu system calling drupal_get_form('menu_view'), but menu_view doesn't return a form structure.

Here's the patch which also adds some more control on the parameters that are passed to menu_view.

drumm’s picture

Status: Needs review » Fixed

Committed to HEAD.

Anonymous’s picture

Status: Fixed » Closed (fixed)
chx’s picture

Assigned: Unassigned » chx
Priority: Normal » Critical
Status: Closed (fixed) » Active

This caused http://drupal.org/node/108587 , I now try to roll this one back and re fix this issue.

chx’s picture

Status: Active » Needs review
FileSize
738 bytes

So the problem was that search/user did not work after this patch, because submitting to search/user meant that the callback for 'search' was called and that had a wired in 'node' callback argument.

Given that search module redirects from the path search to search/node anyways because $type is empty, the fix is quite simple: we just need to remove the callback argument.

ChrisKennedy’s picture

Status: Needs review » Needs work

The patch does restore user searching functionality, but doesn't fix the original issue of displaying an access denied message when an anonymous user without "access profiles" permission manually visits search/user. With the patch a search box and button are displayed that don't return any results (for users without "access profiles" permission).

chx’s picture

Status: Needs work » Needs review

I can not reproduce what ChrisKennedy describes. Have you rebuilt your menu?

chx’s picture

FileSize
89.02 KB

A screenshot.

ChrisKennedy’s picture

I get the access denied error if the user doesn't have "search content" access, but if they have "search content" but not "view user profiles" I just get the non-working search box for search/user. You get "access denied" in the second case too?

This is after deleting cache, cache_menu and cache_page.

chx’s picture

FileSize
1.16 KB

Ah yes. Let's see this one. Note that user_search protects every op with appropriate permission check.

ChrisKennedy’s picture

Status: Needs review » Needs work

Ok this version works correctly.

Two more suggestions: add a comment and tweak the regex to allow module names with numbers.

chx’s picture

Status: Needs work » Reviewed & tested by the community
FileSize
1.24 KB

According to Chris the version already worked correctly, I just added comment and numbers.

ChrisKennedy’s picture

Yup, looks good.

Steven’s picture

Why on earth are you using a regular expression? Just check if the hook returns an empty string or not.

Steven’s picture

Status: Reviewed & tested by the community » Needs work

In fact, I think this is a completely wrong approach. In search_menu():

    $keys = search_get_keys();
    $keys = strlen($keys) ? '/'. $keys : '';
    foreach (module_list() as $name) {
      if (module_hook($name, 'search') && $title = module_invoke($name, 'search', 'name')) {
        $items[] = array('path' => 'search/'. $name . $keys, 'title' => $title,

In other words, if you go to "search/user", a dynamic menu item "search/user" is created (as $keys == '') which should take precedence over "search".

Steven’s picture

Status: Needs work » Needs review
FileSize
1.37 KB

In fact, so search_menu() already checks if the search provider advertises itself. So, the solution is relatively simple... we define an explicit 403 menu item for the case when 1) the search type is supported but 2) the permissions are not set.

The result is that search/user will properly return a 403, and will not require a useless invocation of search_view('user') like in the earlier patch. If you use a non-existant search type, it's as if it wasn't there (e.g. search/foobar == search) and you get the regular node search. This is in line with other drupal behaviour (e.g. admin/foobar == admin).

chx’s picture

FileSize
1.2 KB

I was hovering around this solution for a while but could not get it right. In your wisdom we are humbled.

But still, this can be written in less code.

Steven’s picture

FileSize
1.32 KB

Less code is good, but chx's patch defines a 403 menu item for every module, even those that don't implement search (like color or watchdog). This is bad too, so I re-added the hook check.

And of course, module_list() + module_hook() = module_implements(). Even less code!

Steven’s picture

Status: Needs review » Fixed

Committed with Dries' okay.

dldege’s picture

There is still a bug in search in DRUPAL-5-0 that search forms all submit to search/node. This patch seems related but didn't fix that problem.

See http://drupal.org/node/108925

Thx.

Anonymous’s picture

Status: Fixed » Closed (fixed)