Faulty toolbar subtree hash breaks asynchonous loading of admin menu content [#3130197]

Problem/Motivation

The toolbar subtrees use a hash to determine if access to the subtree should be allowed in ToolbarController::checkSubTreeAccess(). In certain situations the hash does not match, resulting in a JavaScript error and the subtrees not dynamically loading. Temporarily commenting out the hash check from ToolbarController::checkSubTreeAccess() makes the toggles appear again.

The toolbar module relies on _toolbar_get_subtrees_hash function to generate hash based on admin menu subtrees' length. This information is not same when you call it via `hook_toolbar` and via /toolbar/subtrees Ajax route. This difference is due to multiple reasons, few are:
1. If you are on admin pages, subtree uses stable template to generate html while Ajax uses toolbar module template
2. If there is a customization like adding destination to any of the link then hook_toolbar attaches current page url while Ajax attaches /toolbar/subtrees as destination

Steps to reproduce

Set the admin menu in vertical mode.
Make sure to have a site with 2 languages.
Enable the Account administration pages on Language negotiation > Detection and selection.
Set the default language to something non-english.
Set an admin language in your user profile other than the default site language (e.g. english).
Go to a content page in your content language.
Refresh the cache (I did this from the content page with devel module).
Navigate to an admin page.
Observe that the toolbar submenu toggles (blue arrows) are not shown and there is a JavaScript error.

Proposed resolution

The hash is calculated from the rendered subtrees, but if the rendered content changes, the hash no longer matches with the sent one and the access check fails.

Proposed solution is to use $hash = Crypt::hashBase64(serialize(array_keys($subtrees))); instead of $hash = Crypt::hashBase64(serialize($subtrees));. So that the hash is based on structure, rather than content.

Remaining tasks

Understand the purpose of the hash(?). Is it related to CSRF, MITM, permissions?
Agree on an alternative hash input.

User interface changes

Hopefully none

API changes

Probably none

Data model changes

Release notes snippet

Comment	File	Size	Author
#24	3130197-fix_subtree_hash_merge-9-13_10.3.x.patch	1.92 KB	anairamzap
#13	3402656-fix-subtree-hash.patch	1.45 KB	zeeshan_khan
#9	3130197-9.patch	534 bytes	scottatdrake
#3	3130197-2.patch	553 bytes	ash2303

Issue fork drupal-3130197

Show commands

Start within a Git clone of the project using the version control instructions.

Add & fetch this issue fork’s repository

Or, if you do not have SSH keys set up on git.drupalcode.org:

Add & fetch this issue fork’s repository

3130197-faulty-toolbar-subtree changes, plain diff MR !10978
Check out this branch for the first time

Check out existing branch, if you already have it locally
3130197-faulty-toolbar-11.x changes, plain diff MR !11856
Check out this branch for the first time

Check out existing branch, if you already have it locally
3130197-10.4.x changes, plain diff MR !11753
Check out this branch for the first time

Check out existing branch, if you already have it locally

1 hidden branch

11.x

compare

Check out this branch for the first time

Check out existing branch, if you already have it locally

About issue forks

Comments

Comment #1

23 April 2020 at 07:59

ash2303 created an issue. See original summary.

Comment #2

ash2303 commented 23 April 2020 at 08:02

Issue summary:

View changes

Comment #3

ash2303 commented 23 April 2020 at 08:05

Status	File	Size
new	3130197-2.patch	553 bytes

Comment #4

longwave

he/him

English

commented 23 April 2020 at 08:09

Status:

Active

» Needs review

Comment #5

23 April 2020 at 08:09

Version:

8.8.x-dev

» 8.9.x-dev

Drupal 8.8.7 was released on June 3, 2020 and is the final full bugfix release for the Drupal 8.8.x series. Drupal 8.8.x will not receive any further development aside from security fixes. Sites should prepare to update to Drupal 8.9.0 or Drupal 9.0.0 for ongoing support.

Bug reports should be targeted against the 8.9.x-dev branch from now on, and new development or disruptive changes should be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Comment #6

23 April 2020 at 08:09

Version:

8.9.x-dev

» 9.2.x-dev

Drupal 8 is end-of-life as of November 17, 2021. There will not be further changes made to Drupal 8. Bugfixes are now made to the 9.3.x and higher branches only. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Comment #7

23 April 2020 at 08:09

Version:

9.2.x-dev

» 9.3.x-dev

Comment #8

23 April 2020 at 08:09

Version:

9.3.x-dev

» 9.4.x-dev

Drupal 9.3.15 was released on June 1st, 2022 and is the final full bugfix release for the Drupal 9.3.x series. Drupal 9.3.x will not receive any further development aside from security fixes. Drupal 9 bug reports should be targeted for the 9.4.x-dev branch from now on, and new development or disruptive changes should be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Comment #9

scottatdrake commented 22 July 2022 at 19:24

Status	File	Size
new	3130197-9.patch	534 bytes

Rerolled patch against 9.4.x

Comment #10

22 July 2022 at 20:27

Status:

Needs review

» Needs work

The last submitted patch, 9: 3130197-9.patch, failed testing. View results

Comment #11

22 July 2022 at 20:27

Version:

9.4.x-dev

» 9.5.x-dev

Drupal 9.4.9 was released on December 7, 2022 and is the final full bugfix release for the Drupal 9.4.x series. Drupal 9.4.x will not receive any further development aside from security fixes. Drupal 9 bug reports should be targeted for the 9.5.x-dev branch from now on, and new development or disruptive changes should be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Comment #12

22 July 2022 at 20:27

Version:

9.5.x-dev

» 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Comment #13

zeeshan_khan commented 20 March 2024 at 17:52

Status:

Needs work

» Needs review

Status	File	Size
new	3402656-fix-subtree-hash.patch	1.45 KB

This one fixes the problem for me.

Comment #14

smustgrave commented 20 March 2024 at 18:00

Status:	Needs review	» Needs work
Issue tags:		+Needs tests, +Needs issue summary update

Will need test coverage showing the problem.

Issue summary is also incomplete, recommend using the standard issue template

Comment #15

neograph734

he/him

Netherlands

commented 20 March 2024 at 19:35

Title:	Faulty Toolbar Subtree Hash	» Faulty toolbar subtree hash breaks asynchonous loading of admin menu content
Issue summary:	View changes

I have updated the IS and merged it with the content of #3402656: Toolbar causes a Javascript error if the subtrees content changes between page loads.

The patch from #2 and #9 does not appear to work. It makes the issue go away, but when a new module is installed, the menu does not update and the new links don't become visible. So the hash might be too generic(?).

The patch from #13 does not appear to do anything. The toggles are still missing and the error is still shown?

Should we perhaps try hasing the raw menu tree structure or something instead?

Comment #16

neograph734

he/him

Netherlands

commented 20 March 2024 at 19:38

Issue summary:

View changes

Comment #17

matthiasm11 commented 1 June 2024 at 21:05

Combining the patches from #9 to generate the hash and #13 to fix the caching seems to fix the issue.
Disabling a module makes the corresponding menu item disappear, enabling the module again makes it appear again.
No console errors anymore. I've not investigated this any further.

Comment #18

milos.kroulik commented 5 August 2024 at 14:00

I can confirm observations made in #17. So I'm going to create a combined patch.

Comment #19

ericdsd commented 22 January 2025 at 16:47

Hi milos.kroulik, did you create a combine patch for it yet ?

Note i'm still experiencing it on core 10.3.11.

Comment #20

22 January 2025 at 17:25

sleitner made their first commit to this issue’s fork.

Comment #21

22 January 2025 at 17:44

sleitner changed the visibility of the branch 11.x to hidden.

Comment #22

22 January 2025 at 17:47

sleitner opened merge request !10978

Comment #23

sleitner commented 22 January 2025 at 18:10

ToolbarAdminMenuTest test still needs to be updated

Comment #24

anairamzap

she/her

Spanish

🇦🇷 ✌🏽 Buenos Aires

commented 19 February 2025 at 15:54

Status	File	Size
new	3130197-fix_subtree_hash_merge-9-13_10.3.x.patch	1.92 KB

I've created a patch that merges the ones in #9 and #13 based on core 10.3.x

Leaving this as "Needs work" since a test update is still missing

Comment #25

frontmobe

Dutch

Amsterdam

commented 20 February 2025 at 16:07

Thanks @anairamzap, I can confirm that your patch also cleanly applies to Drupal 10.4.3.

Comment #26

7 April 2025 at 09:05

carlos romero made their first commit to this issue’s fork.

Comment #27

7 April 2025 at 09:05

carlos romero opened merge request !11753

Comment #28

carlos romero commented 7 April 2025 at 09:07

Status:

Needs work

» Needs review

Comment #29

carlos romero commented 7 April 2025 at 09:07

Hi, I've forked, applied the change to the 10.4.x branch, and submitted the PR.

Best regards.

Comment #30

smustgrave commented 7 April 2025 at 12:30

Status:

Needs review

» Needs work

Have not reviewed but changes need to be against 11.x as that’s the development branch

Comment #31

16 April 2025 at 13:06

tinto changed the visibility of the branch 11.x to active.

Comment #32

16 April 2025 at 13:07

tinto changed the visibility of the branch 11.x to hidden.

Comment #33

16 April 2025 at 13:56

tinto opened merge request !11856

Comment #34

tinto

Amsterdam

commented 16 April 2025 at 14:06

Status:	Needs work	» Needs review
Issue tags:		+ddd2025

Rebased to 11.x

Greetings from the contrib room at Drupal Developer Days 2025 in Leuven, Belgium.

Comment #35

smustgrave commented 17 April 2025 at 00:40

Status:

Needs review

» Needs work

Hello!

New parameter probably should be typehinted and parameter doc added

Also will most likely need test coverage

Comment #36

sleitner commented 20 July 2025 at 16:27

The MR doesn't fix the problem when installing/uninstalling modules which only add a sub menu item e.g. "User interface translation" for locale module.

The keys of the array do not change, so the hash does not change either. Maybe the installed modules list should be in in the hash input, too?

Comment #37

20 July 2025 at 16:27

Version:

11.x-dev

» main

Drupal core is now using the main branch as the primary development branch. New developments and disruptive changes should now be targeted to the main branch.