Support from Acquia helps fund testing for Drupal Acquia logo

Comments

floretan’s picture

Status: Active » Needs review
FileSize
1.94 KB

To reproduce:

0. Enable the search module.
1. Set all search factor weights to 0 (this disables all of them).
2. Create a page node with the keyword "drupal".
3. Run cron.
4. Search for the keyword "drupal". Division by zero error in node.module (in the implementation of hook_search).

The node search code already defaults to using keyword relevance as the default search factor when none are specified:

$select2 = (count($ranking) ? implode(' + ', $ranking) : 'i.relevance') . ' AS score';

But count($ranking) is only zero when all search factors have a zero weight, meaning that their total weight is zero, which leads to the division by zero a little further in the code.

This patch doesn't change the algorithm, it just changes the code to use a proper if-statement. When using the default search factor, the total weight can safely be set to 1 (for those who are into mathematics and set theory, we can do this because 1 is the identity element of real numbers under multiplication, but that's beyond the scope of this patch).

Senpai’s picture

Status: Needs review » Reviewed & tested by the community

Tested and Working As Intended. I followed these steps:

1. Enabled the search module.
2. Set all search factor weights to 0.
3. Created a page node with the word "drupal" as the body text.
4. Created an article node with the word "drupal" as the body text.
5. Ran cron using devel.mod.
6. Searched for the keyword "drupal".
7. Received both pieces of content in the resulting list, with no divide-by-zero errors present in drupal_set_message().

Dries’s picture

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

Committed to CVS HEAD. Thanks.

The patch did not apply against DRUPAL-6 so it will need a reroll.

Dries’s picture

Committed to CVS HEAD. Thanks.

The patch did not apply against DRUPAL-6 so it will need a reroll.

floretan’s picture

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

Re-rolled for drupal 6. The only change was the spacing around the concatenation operator.

Gábor Hojtsy’s picture

Status: Reviewed & tested by the community » Fixed

Thanks, committed to Drupal 6!

Anonymous’s picture

Status: Fixed » Closed (fixed)

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

catch’s picture

Version: 6.x-dev » 5.x-dev
Status: Closed (fixed) » Patch (to be ported)

Still a valid bug in D5.

floretan’s picture

Status: Patch (to be ported) » Reviewed & tested by the community
FileSize
2.01 KB

Patch still applied with some offset. Re-rolling without changes.

drumm’s picture

Status: Reviewed & tested by the community » Fixed

Committed to 5.x.

Anonymous’s picture

Status: Fixed » Closed (fixed)

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