Just upgraded to 6.x-1.2 and now when a Mark as spam link is clicked on a comment we get a page not found error.
Here is the path it is trying to access:

/spam/node/11020/spam?token=5f3fa19a93cf36ce813cbf58068cf8d1

Comments

killes@www.drop.org’s picture

killes@www.drop.org commented

that's strange, can you go to the admin/build/modules page and save that once?

killes@www.drop.org’s picture

killes@www.drop.org commented

and are you sure you got the link off a comment? that looks like a node link to me.

mr.j’s picture

mr.j commented
Title: Mark comment as spam = page not found error » Mark node as spam = page not found error

I have saved modules and emptied caches.

And you are right - it is a node link. The same thing happens with mark as spam on comment links too. Apologies for not making that clear.

mr.j’s picture

mr.j commented

I diff'd against the last version that we had that was working and the only difference I can see in the spam_menu function is the addition of "_callback" to the function names. So I checked the menu_router table and found these records which both look right:

Path: spam/%/%/not_spam
Callback: spam_mark_as_not_spam_callback

Path: spam/%/%/spam
Callback: spam_mark_as_spam_callback

I looked at the token checking function and I can't see how that would generate a 404.
I am running as the admin user so permissions should not be an issue.

I have reproduced it in my dev environment so I am trying to debug the problem now.

mr.j’s picture

mr.j commented

Ok the problem as far as I can tell:

menu_get_item() is always returning FALSE instead of a valid menu router array for the relevant spam / not-spam menu handlers. See _menu_translate() call on line 317 of menu.inc which returns FALSE.

This can be traced to spam_mark_load() always returning FALSE as it tries to call "comment_spamapi" (and presumable "node_spamapi") which does not exist as a function, through the spam_invoke_module() call at line 584 of spam.module.

Reading the code, I assume these functions should have been included by the spam_init() function as it scans the modules subdirectory. But debugging shows it is called after menu_get_item(). The old module called spam_init_api() unconditionally, so it looks like that change is what has caused the problem.

killes@www.drop.org’s picture

killes@www.drop.org commented

"The old module called spam_init_api() unconditionally, so it looks like that change is
what has caused the problem. "

There is no function spam_init_api() that I can find. There is only spam_init() and there are no conditions imposed on it being called. It is called by Drupal core.

poakpong’s picture

poakpong commented

Me too. Page not found

mr.j’s picture

mr.j commented

spam_init_api() was in 6.x-1.0 and was called in global scope at the top of the module, on every page load I guess. It looks like the function was renamed to spam_init in the latest release, presumably to use hook_init. But it is called too late to include the code required by spam_mark_load().

killes@www.drop.org’s picture

killes@www.drop.org commented

I can reproduce this if I comment out the require call inside the hook_init implementation. I am still not getting why the init hook wouldn't be called before the menu handler...

killes@www.drop.org’s picture

killes@www.drop.org commented

Can you reproduce this is you install a clean 1.2 version on another Drupal site where the module was previously not?

killes@www.drop.org’s picture

killes@www.drop.org commented

Also, can you try switching off other modules?

sinmao’s picture

sinmao commented

Me three :) Getting page not found for comment after clicking on mark as spam.

AlexisWilke’s picture

AlexisWilke commented

I'm running 1.x-dev and had the version before the security update. Things also work fine for me. I have not seen any page disappear on me...

mr.j’s picture

mr.j commented

Ok looks like I have found a conflict with the custom_breadcrumbs module.

custom_breadcrumbs_init() calls
custom_breadcrumbs_set_menu_breadcrumb() which calls
menu_get_item()

before spam_init() is called.

Disabling "Use the menu structure to set the breadcrumb trail" in custom breadcrumb settings fixes the problem for me, and makes no difference on my site. But this may cause other sites to lose some of the custom breadcrumbs functionality.

Alternatively you could probably alter the weight of the custom_breadcrumb or spam module so that custom_breadcrumb hook_init code runs after spam's hook_init code.

Not sure whether this belongs in the spam issues queue or custom_breadcrumbs now.

sinmao’s picture

sinmao commented

Hmm...

I have that unchecked and still have the problem.

AlexisWilke’s picture

AlexisWilke commented

sinmao,

You may have yet another module that calls the "wrong" menu function. Note that on my side I do use the custom_breadcrumb, but indeed, that very flag is off on all my sites.

Thank you.
Alexis

mkmk’s picture

mkmk commented

I am getting page not found too.

mkmk’s picture

mkmk commented

I don't use the custom_breadcrumbs module but get this error too. This used to work.

Another thing I noticed is that under admin/reports/spam (The Spam Module Logs) I see entries like:

type id datesort icon message user operations
comment 0 2011-06-25 06:52 final average(1) Anonymous detail
comment 0 2011-06-25 06:43 final average(1) Anonymous detail
comment 0 2011-06-25 05:30 final average(1) Anonymous detail
comment 9343 2011-06-25 04:33 final average(1) Anonymous detail
comment 0 2011-06-25 04:33 final average(1) Anonymous detail
comment 0 2011-06-25 04:32 final average(1) Anonymous detail

...meaning, the id has always been 0, but for some comments there is a non-zero (9343) id.

AlexisWilke’s picture

AlexisWilke commented

mkmk,

I noticed that identifier problem. I'm not too sure why it happens...

Now, have you tried with the development version to see whether the problem was fixed in 6.x-1.x-dev?

Thank you.
Alexis

mkmk’s picture

mkmk commented

Will running the development version break the existing version?

mkmk’s picture

mkmk commented

I tried the development version and the problem is not fixed.

mkmk’s picture

mkmk commented

I also looked at http://drupal.org/node/1183540 and disabled the Duplicate Filter. Still, marking a comment as spam brings up a Page Not Found error.

AlexisWilke’s picture

AlexisWilke commented

If you have version 1.2 then testing with the development version won't change anything. There weren't any changes to the database tables so it is safe to use to test.

Thank you.
Alexis

silverwing’s picture

silverwing commented

spam clean-up (excuse the noise)

killes@www.drop.org’s picture

killes@www.drop.org commented

Maybe spamming the spam module issue queue should eb seen as a sign to actually use the module :p

Regarding this issue: I am not sure that calling menu_get_item in hook_init should be considered good practise. I am also not sure that the loading of the .inc files pratised by spam.module is good practise...

killes@www.drop.org’s picture

killes@www.drop.org commented

I've now moved the loading of the .inc files into a spam.inc file which is loaded by the menu when accessing the "mark as" links. This requires a rebuild of the menu (go to admin/build/modules and click "save").

I'll push this later today.

killes@www.drop.org’s picture

killes@www.drop.org commented

The above approach didn't work, I am now loading the files from the _load function directly.

killes@www.drop.org’s picture

killes@www.drop.org commented

this is now in git on d.o, please test. There are a number of unrelated changes which don't mind testing too. ;)

mkmk’s picture

mkmk commented

It works for me now. Thanks a lot!!

AlexisWilke’s picture

AlexisWilke commented

Killes,

I think that the ugly includes were remnants from Drupal 5.x.

Thank you for all your good work here!
Alexis

killes@www.drop.org’s picture

killes@www.drop.org commented
Version: 6.x-1.2 » 6.x-1.x-dev
Status: Active » Fixed

no problem, this is actually paid for work. ;)

I am marking this fixed now. I am working to integrate privatemsg now, then I'll roll a release candidate so that some testing can be done.

Status: Fixed » Closed (fixed)

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