Come together with the global Drupal community in Rotterdam, 28 Sept – 1 Oct 2026. Sessions, contribution, connection, and Early Bird savings until 8 June.Problem/Motivation
Bug occurs when access is denied for the current user to a node in a book hierarchy
When visiting a child page there is an undefined index for title on line 425 of book.module
The undefined index is in function template_preprocess_book_navigation()
In Drupal 7 the return value of _menu_link_translate() is FALSE if access is denied
In Drupal 8 \Drupal\book\BookManagerInterface::loadBookLink always returns an array, so it's never empty unless the node doesn't exist.
In Drupal 8 we need to check the value of the access key in the array.
Proposed resolution
Fix logic of line 418 in book.module.
Add a test (?)
Remaining tasks
patch & review
User interface changes
n/a
API changes
n/a
Data model changes
n/a
| Comment | File | Size | Author |
|---|---|---|---|
| #34 | interdiff_32_34.txt | 829 bytes | anmolgoyal74 |
| #34 | undefined_index_in-2670150-34.patch | 2.31 KB | anmolgoyal74 |
| #32 | undefined_index_in-2670150-32.patch | 2.3 KB | kasey_mk |
| #23 | interdiff-18-23.txt | 769 bytes | mohit_aghera |
| #23 | undefined_index_in-2670150-23.patch | 2.31 KB | mohit_aghera |
Issue fork book-2670150
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
- 2670150-undefined-index-in
changes, plain diff MR !35
Comments
Comment #2
pwolanin commentedHere's the fix (but no test yet)
Comment #3
davidhernandezThe situation results in a big ugly error. The patch presumably fixes it. But, what would be tested? In the current situation, the child node renders successfully.
Comment #4
pwolanin commentedI think the test could simply be a regression test and reproduce the current error and fail as test-only and pass with the patch
Comment #5
harika gujjula commentedComment #6
heykarthikwithuIMO current issue #2670150: Undefined index in template_preprocess_book_navigation() when parent node access is denied depends on #2675580: Required $operation parameter is missing in the book_node_view() in book.module
Comment #8
wheatpenny commentedComment #9
pwolanin commentedComment #10
wheatpenny commentedUploading a draft of the test only patch. Needs to be reviewed and modified.
Comment #11
wheatpenny commentedIn order to help the next person working on this, the tests above result in the following error messages:
"( ! ) Fatal error: Call to a member function id() on a non-object in /var/www/nola/core/modules/book/src/Tests/BookTest.php on line 773"
Comment #13
catchComment #14
mohit_aghera commentedComment #16
mohit_aghera commentedFixing issues in test case.
Comment #18
mohit_aghera commentedComment #19
kgoel commentedGoing to review the patch
Comment #20
kgoel commentedTest patch in https://www.drupal.org/node/2670150#comment-11399301has test overage without fix in https://www.drupal.org/node/2670150#comment-10863592. Test patch should have failed but instead it passes. Issue needs work.
Comment #22
mohit_aghera commentedAdding fix for test coverage.
Comment #23
mohit_aghera commentedFixing diff issue.
Comment #24
mohit_aghera commentedComment #30
ptmkenny commentedRemoving "Needs tests" tag because the latest patch has a test.
Comment #32
kasey_mk commentedRe-roll. Sorry for the lack of an interdiff; ran into tool trouble and out of time.
Comment #34
anmolgoyal74 commentedFixed non-existent service entity.manager.
Comment #41
quietone commentedThis extension is being deprecated, see #3376070: [Meta] Tasks to deprecate Book module. It will be removed from core and moved to a contrib project, #3376101: [11.x] [Meta] Tasks to remove Book.
This is now Postponed. The status is set according to two policies. The Remove a core extension and move it to a contributed project and the Extensions approved for removal policies.
This issue may be re-opened if it can be considered critical, If unsure, re-open the issue and ask in a comment.
Comment #42
smustgrave commentedMoving to contrib.
Comment #43
smustgrave commentedComment #45
smustgrave commentedActually wonder if this is still needed for 2.0.x?
Comment #47
smustgrave commented