Having just upgraded from 6.x-1.x-dev to 6.x-1.7 I am getting sql errors on ever forum page.

  • warning: array_fill(): Number of elements must be positive in /var/www/includes/database.inc on line 252.
  • warning: implode(): Invalid arguments passed in /var/www/includes/database.inc on line 252.
  • warning: array_keys() expects parameter 1 to be array, null given in /var/www/modules/user/user.module on line 514.
  • user warning: You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near ')' at line 1 query: SELECT p.perm FROM role r INNER JOIN permission p ON p.rid = r.rid WHERE r.rid IN () in /var/www/modules/user/user.module on line 514.

Not sure where to go from here...
Thankfully my forum is still in development stages.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

salvis’s picture

I don't see this there. Can you install the Devel module, enable the backtrace error handler and post a screenshot of the call stacks that you get, please?

broncomania’s picture

FileSize
66.79 KB

Subscribe, same problem here. Update from 1.5 to 1.7

tetherow’s picture

The issue seems to be that _forum_access_access_any_forum is getting called with $account = 'access content' instead of being called with user object.

salvis’s picture

Status: Active » Postponed (maintainer needs more info)

@broncomania and @tetherow: You are reporting a completely different error in a completely different location.

Please open your own new issue (and post the screenshot again). And also give the path (URL) of the page where you get this! Clear your menu cache. Search through your entire code base for all occurrances of '_forum_access_access_any_forum'. In FA it's used only in forum_access_menu_alter() and attached to the forum path and $items['forum']['access arguments'] is cleared, as it should be. _forum_access_access_any_forum() cannot be called with 'access content' based on this. Do not reply here but open your own issue! In your screenshot, expand the top items so that we can see the arguments being passed.

 

My request for a backtrace to the "SQL error" in this issue still stands.

aburnsni’s picture

FileSize
34.6 KB
92.35 KB

Not sure what bits you needed expanded but here are the initial screenshots of the backtrace.
Let me know what else you need.

tetherow’s picture

The error messages I get are exactly the same as the error messages listed above, including the user warning SQL statement, so I'm not sure what I said that led you to the belief that this is a different error. It started occuring when I upgraded forum_access from 6.x-1.5 to 6.x-1.7.

I have done both a drush cc all and a drush cc menu and the problem still persists. If I edit the source and remove the $account parameter from line 628 (if (!user_access('access content', $account)) {) then the problem goes away.

I put in a simple debug statement to display the value of account when _forum_access_access_any_forum was called and the value being passed is 'access content'.

I haven't worked much with hook menu, and I'm not finding anything explicitly defining it, but by default the value of 'access callback' is user_access and the value of 'access arguments' is an array containing the permission to be checked.

So if you are modeling _forum_access_access_any_forum off of user_access shouldn't it be defined as:
function _forum_access_access_any_forum($string, $account=NULL) {

to mirror the arguments passed by default to user_access ?

salvis’s picture

FileSize
26.77 KB

@tetherow:

I'm not sure what I said that led you to the belief that this is a different error.

#2 and #3 lack any evidence that they might be the same issue as the OP. The OP does not mention _forum_access_access_any_forum() at all and the comments don't mention anything that the OP does.

The error messages I get are exactly the same as the error messages listed above

What "above"? Please don't say things like "above". Always refer to specific comments numbers, spelled-out paths, etc. I have no way to infer what may be obvious to you but unstated.

@aburnsni:
Thank you for the screenshots. I can now see that it's effectively the same issue. Your site may not have displayed the notices before enabling Devel's error handler, but the first message is often the most important one, because it points to the initial problem. Later messages are often just follow-up errors.

@tetherow:

I put in a simple debug statement to display the value of account when _forum_access_access_any_forum was called and the value being passed is 'access content'.

That's good information. Now we need to find out why this is so. _forum_access_access_any_forum() should usually not be called with any argument, and no, it's not modeled after user_access().

Please add
ddebug_backtrace();
at the top of _forum_access_access_any_forum(). On the front page of my test site this gives me the following:
Backtrace

This call determines whether the "Forums" menu item (blue marks) should be visible for the current user. Note how _forum_access_access_any_forum() is called without any argument (green marks), which is exactly what's expected given

function forum_access_menu_alter(&$items) {
  if (!empty($items['forum'])) {
    // ...
    $items['forum']['access callback'] = '_forum_access_access_any_forum';
    unset($items['forum']['access arguments']);
  }
}

Please post a screenshot of your backtrace, with the same elements expanded, and state what path (URL) you're showing.

aburnsni’s picture

FileSize
66.29 KB

Hopefully this is what you need.
the URL is http://www.flemingfulton.org.uk/forums/parents - but is currently not visible by anonymous users.

Andrew

salvis’s picture

@aburnsni: Thank you for your reply.

forums/parents? That is not a standard path. Is it a path alias? For what standard path? Does the problem also occur when you go to the standard path that the alias maps to?

Your screenshot shows _forum_access_access_any_forum() being called without any argument (0 elements). That is not the call where 'access content' is supposedly passed. There must be other calls, or you're not getting the error on that page, or...?

aburnsni’s picture

FileSize
50.97 KB

Perhaps I had misunderstood your

Please add
ddebug_backtrace();
at the top of _forum_access_access_any_forum()

With it removed I get the following on the URL /forum/60
This php/backtrace stuff is all a bit beyond my understanding so bear with me.

salvis’s picture

Title: SQL errors moving from 6.x-1.x-dev to 6.x-1.7 » Advanced Forum breaks Forum Access 6.x-1.7
Project: Forum Access » Advanced Forum
Version: 6.x-1.7 » 6.x-1.x-dev
Priority: Normal » Major
Status: Postponed (maintainer needs more info) » Active

Ah, finally... Someone could have mentioned that they're using advanced_forum... or that they're looking at forum/view...

This is a bug in AF: AF must not pass anything to _forum_access_access_any_forum().

Please set Version to the version of Advanced Forum that you're using — I sure hope you're not using an ALPHA version of AF without telling me about it...

Michelle’s picture

Where are you getting that this is an AF bug? AF doesn't do much of anything with Forum Access. Because others have been working on the module, I did a grep on "forum_access" on both 6.x branches to be sure nothing was added that I wasn't aware of and I get this:

temp/62/advanced_forum/advanced_forum.module: if (module_exists('forum_access')) {
temp/62/advanced_forum/advanced_forum.module: if (!forum_access_access($tid, 'create', NULL, TRUE)) {

One is checking if the module exists and the other is calling a public function, not the private one mentioned in this issue.

Michelle

salvis’s picture

Hi Michelle

The forum/view menu router item shown in the screenshot of #10 (for advanced_forum_page()) must have a NULL 'access arguments' array item. It's passing array('access content') instead.

This must be in your hook_menu() or hook_menu_alter() implementation.

Maybe it's AF 6.x-2.x-dev?

Thanks for looking into it!

aburnsni’s picture

Version: 6.x-1.x-dev » 6.x-2.x-dev

:Hangs head in shame:
OK it's AF 2.x-dev. I don't understand what is being said in #12, #13 but I realise that using dev (or alpha) versions can often break things.
I assumed that since the break corresponded with the upgrade of forum_access from dev to stable that that was where the problem lay.
What else can I do to help find the route of this problem?
Thanks for the help so far.

mcdruid’s picture

salvis, here's a snippet from a git blame of advanced_forum.module:

1955908b (Michelle    2010-11-16 15:55:07 +0000   41)   if (variable_get('advanced_forum_add_local_task', TRUE)) {
10e45f28 (Michelle    2010-03-09 04:34:49 +0000   42)     $items['forum/view'] = array(
863e8571 (Michelle    2011-03-04 23:18:24 -0500   43)       'title' => 'View Forums',
10e45f28 (Michelle    2010-03-09 04:34:49 +0000   44)       'page callback' => 'advanced_forum_page',
10e45f28 (Michelle    2010-03-09 04:34:49 +0000   45)       'access arguments' => array('access content'),
10e45f28 (Michelle    2010-03-09 04:34:49 +0000   46)       'type' => MENU_DEFAULT_LOCAL_TASK,
10e45f28 (Michelle    2010-03-09 04:34:49 +0000   47)       'weight' => -100,
3da823bd (lynn        2011-08-02 00:49:04 +0100   48)       'file' => 'includes/core-overrides.inc'
10e45f28 (Michelle    2010-03-09 04:34:49 +0000   49)     );
10e45f28 (Michelle    2010-03-09 04:34:49 +0000   50)   }

...so AF's hook_menu does indeed have 'access arguments' => array('access content'), but it appears that this has been there since at least March 2010.

Forgive my ignorance, but why should AF "have a NULL 'access arguments' array" here? surely it's correct that AF should be doing some of its own (basic) permissions checking?

Michelle’s picture

Based on #15, this doesn't sound like a bug in AF. FA shouldn't be assuming that there are no other access control checks on a page. In fact, here is core forum:

  $items['forum'] = array(
    'title' => 'Forums',
    'page callback' => 'forum_page',
    'access arguments' => array('access content'),
    'type' => MENU_SUGGESTED_ITEM,
    'file' => 'forum.pages.inc',
  );

and that has an access argument as well. So how does FA even work with core?

Michelle

tetherow’s picture

Sorry for the poor quality of the bug report, it was a long week and under a deadline. You are correct, the issue appears to be a change in Forum Access and not Advanced Forum.

It looks like a lot has gone on in this issue while I was out for a long weekend. Do you still want me to do the backtrace or are we past that point on this? If there is anything I can do to help (if the issue even belongs here) let me know.

salvis’s picture

Sorry about taking so long to reply, I've been away.

We have a weakness of the inheritance rules here.

forum has 'access arguments' => array('access content') and defaults to user_access().
FAAF's forum/view has 'access arguments' => array('access content'), too, and inherits user_access() from its parent.

However, FA replaces forum's access specs with

    'access callback' => '_forum_access_access_any_forum',
    'access arguments' => NULL,    // actually unset()

This causes AF to inherit '_forum_access_access_any_forum' and we get the bad _forum_access_access_any_forum('access content') call.

This has been invalid since the code in #15 was committed, but it didn't matter because _forum_access_access_any_forum() didn't have any parameter and simply ignored the bogus string.

Starting with 6.x-1.7 FA now has an optional $account parameter in _forum_access_access_any_forum(), and getting 'access content' passed in is not negligible anymore.

So, to sum it up, it's a recent change in FA that has brought the issue to light, but it needs to be fixed in AF: AF needs to set 'access callback' => 'user_access' in addition to 'access arguments'. There's nothing that FA could do about this situation.

I can't claim that I was aware of this trap before and that I would have coded it differently, but it's now obvious that it's an error for AF to assume that it will inherit 'user_access'.

BTW, this is true for all versions of Drupal: unless you're on a top-level path, you must always set both the callback and the arguments, because you can't rely on what you inherit. I'm sure there are lots of these landmines out there...

@aburnsni and @tetherow: It turns out that AF 6.x-1.x causes the exact same misbehavior. But knowing that another contrib was installed that affected the same pages (or even created them in the first place!), or knowing what path (forum/view) we're talking about, would have made this much shorter and less painful. There's nothing more for you to do, thank you for your help!

Michelle’s picture

Wow, that's a crazy one... I think it's pushing the limits of "bug" for something that works just fine unless another module comes along and mucks with things. But I can see it makes sense to make the change in AF now that I understand the issue. I didn't know anything about this crazy inheritance when I wrote it; I was just following what core did.

Michelle

salvis’s picture

I agree, but allowing "another module to come along and muck with things" is the basic principle of Drupal. It just doesn't work right here with the (wrong) expectations that we get from the inheritance mechanism.

I'm going to look through all of my modules for similar omissions.

mcdruid’s picture

Status: Active » Fixed

Thanks for the excellent explanation salvis.

@@ -42,6 +42,7 @@ function advanced_forum_menu() {
     $items['forum/view'] = array(
       'title' => 'View Forums',
       'page callback' => 'advanced_forum_page',
+      'access callback' => 'user_access', 
       'access arguments' => array('access content'),

Committed to 6.x-2.x branch.

salvis’s picture

Thanks, mcdruid!

Hmm, I've been thinking about this some more, and it might actually be better to remove 'access arguments' rather than adding 'access callback'. Your intention is to get the same access check for forum/view that forum has, and that's what you get when you omit and thus inherit BOTH keys.

The reason for FA to "muck" with forum's access is that the "Forum" link should not appear if the user (especially Anonymous!) has no access to any forum. I would guess that forum/view shouldn't be accessible either if forum isn't.

RedRat’s picture

JFYI, I have tested both solutions from #21 and #22, and in both cases error is gone. But development release of 15 November still has it. :-(

salvis’s picture

Thanks RedRat!

forum/view is the MENU_DEFAULT_LOCAL_TASK — it shouldn't specify its own access anyway. Nor its own callback. The default local task should inherit almost everything from its parent, because it must be the same conceptually. See the example on hook_menu() and MENU_DEFAULT_LOCAL_TASK.

I've installed AF and confirmed, that the access check is correctly inherited from the parent if you remove the two items.

mcdruid’s picture

salvis - that makes sense too - I'll get the callback and arguments removed and committed later on today.

salvis’s picture

Thank you, mcdruid. Can you do the same in 7.x-2.x, too?

aburnsni’s picture

Thanks guys, I don't understand the details but, my big red errors at the top of the page are gone in 6.x-2.x-dev. :-)

Michelle’s picture

Status: Fixed » Active

Once it's in 6.x, just set the version to 7.x and troky should see it.

Thanks,

Michelle

troky’s picture

Status: Active » Fixed

Fixed and committed to 7.x.

Michelle’s picture

Thanks!

Michelle

mcdruid’s picture

salvis’s picture

Great, thanks to all!

Status: Fixed » Closed (fixed)

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

dunx’s picture

For those that don't really want to run -dev on a live system and whilst we wait for a 6.x-2.0-alpha5, simply comment out line 44 of advanced_forum.module.

// 'access arguments' => array('access content'),

That seems to fix the problem for me.

Ingumsky’s picture

Wow. That was a fantastic read mates. And I'm not joking. That was like a crime story with brilliant plot! Thank you... And my issue has been solved in the process :)