if ($result = db_query("SELECT nid, bid FROM {book} WHERE nid = %d", $nid)) {
    $book = db_fetch_object($result);
    if ($book->nid == $book->bid) {
      $return += BOOK_MANAGER_IS_BOOK;
    }
    $return += BOOK_MANAGER_IS_IN_BOOK;

So, any time you get a NULL result from the query, your NULL nid matches your NULL bid and your non-book node is counted as a book by book manager.

Comments

evoltech’s picture

evoltech commented
Assigned: Unassigned » evoltech

I will be submitting a patch for this soon.

evoltech’s picture

evoltech commented
Status: Active » Postponed (maintainer needs more info)

I don't see how this function could be called with a $nid == NULL. The only place it is called from is book_manager_nodeapi when a node is in the 'load' state. Please provide more feedback on how this issue can be triggered.

jody lynn’s picture

jody lynn
she/her
Primary language English
commented

No, the function is not called with a null nid, but it can be called by a node which is not in a book. When that happens, the query returns a null result, meaning that $book->nid and $book->bid are both NULL, and the non-book node is now assigned the status of a book.

The real reason I opened this ticket though, is not so much to fix it, but as a warning to others that this module has critical bugs and should not be used. I've had to spend extensive time debugging this module and have come to the conclusion that you'd be much better served with a complete rewrite than trying to fix it. It's not just that it's completely broken and buggy, but it's also extremely over-complicated code for the feature it's delivering and a birds nest of permissions and UI mess. I'm sorry to be rude, but I feel I'm doing an extreme disservice to other people by not sharing this opinion. The best thing that could be done with this module IMO is for it to be deleted.

evoltech’s picture

evoltech commented
Assigned: evoltech » Unassigned
marcp’s picture

marcp commented
Title: Book_manager_status logic problem » book_manager_status sets the wrong status for nodes that are not books
Assigned: Unassigned » marcp
Status: Postponed (maintainer needs more info) » Active

Thanks for this bug report -- it's a live one! We always need to check db_fetch_object()'s return value to see if it's FALSE.

marcp’s picture

marcp commented
Status: Active » Fixed

Committed. Thanks for reporting this. We hope to remove the WARNINGS that are placed on all the sharp edges of this module soon.

Status: Fixed » Closed (fixed)

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