Posted by Gerhard Killesreiter on April 29, 2008 at 10:24am
| Project: | Drupal core |
| Version: | 5.x-dev |
| Component: | node.module |
| Category: | bug report |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | closed (fixed) |
Issue Summary
Found by Robert
| Project: | Drupal core |
| Version: | 5.x-dev |
| Component: | node.module |
| Category: | bug report |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | closed (fixed) |
Found by Robert
Comments
#1
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).
#2
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().
#3
Committed to CVS HEAD. Thanks.
The patch did not apply against DRUPAL-6 so it will need a reroll.
#4
Committed to CVS HEAD. Thanks.
The patch did not apply against DRUPAL-6 so it will need a reroll.
#5
Re-rolled for drupal 6. The only change was the spacing around the concatenation operator.
#6
Thanks, committed to Drupal 6!
#7
Automatically closed -- issue fixed for two weeks with no activity.
#8
Still a valid bug in D5.
#9
Patch still applied with some offset. Re-rolling without changes.
#10
Committed to 5.x.
#11
Automatically closed -- issue fixed for two weeks with no activity.