Contrib.social

Path comparison (e.g. for block visiblity) doesn't work for aliased internal paths with capital letters (block_block_list_alter() in D7)

Open on Drupal.org →
Created on 17 November 2015, over 8 years ago
Updated 12 February 2024, 6 months ago

Problem/Motivation

Path comparison (e.g. for block visiblity) doesn't work for aliased internal paths / routes with capital letters for D7 (see block_block_list_alter() function). And D8 + 9 are affected, too.

Both sides of the string comparison must be lowercased, but the (system) path isn't lowercased.

Steps to reproduce

  1. Create an internal path (e.g. for a Views page or with hook_menu()) that contains at least 1 capital letter.
  2. Create an URL alias for that path.
  3. Enter the internal path as visibility page for any block and check that the block visibilty doesn't work as expected.

Proposed resolution

For the comparison of the aliased path everything is fine. But in the 2nd comparison with the internal path there is a missing drupal_strtolower() call for the internal path ($_GET['q']).

For D9 the file /core/modules/system/src/Plugin/Condition/RequestPath.php must be changed from
return $this->pathMatcher->matchPath($path_alias, $pages) || (($path != $path_alias) && $this->pathMatcher->matchPath($path, $pages));
to
return $this->pathMatcher->matchPath($path_alias, $pages) || (($path != $path_alias) && $this->pathMatcher->matchPath(mb_strtolower($path), $pages));

The D7 bugfix is a change in the https://api.drupal.org/api/drupal/modules%21block%21block.module/functio... file from
$page_match = $page_match || drupal_match_path($_GET['q'], $pages);
to
$page_match = $page_match || drupal_match_path(drupal_strtolower($_GET['q']), $pages);

Remaining tasks

  • review existing patch
  • backport for D7

User interface changes

none

API changes

...?

Data model changes

...?

Release notes snippet

...?

Original report by [M_Z]

Short description of the problem: both sides of the string comparison must be lowercased, but the (system) path isn't lowercased…

D7 + D8 are affected by this bug.

-----

API page: https://api.drupal.org/api/drupal/modules%21block%21block.module/functio...

When the path patterns are compared with the current path the block_block_list_alter() function says:

// Compare the lowercase internal and lowercase path alias (if any).

For the comparison of the aliased path everything is fine. But in the 2nd comparison with the internal path there is a missing drupal_strtolower() call for the internal path ($_GET['q']).

So the bugfix would be an easy replacement of the line
$page_match = $page_match || drupal_match_path($_GET['q'], $pages);
with
$page_match = $page_match || drupal_match_path(drupal_strtolower($_GET['q']), $pages);
.

Or is there any mechanism that "auto"-lowercases $_GET['q']? I don't know such a mechanism...

To reproduce the bug create an internal path (e.g. for a Views page or with hook_menu()) that contains at least 1 capital letter. Create an URL alias for that path. Enter the internal path as visibility page for any block and check that the block visibilty doesn't work as expected.

🐛 Bug report
Status

Needs work

Version

11.0 🔥

Component
Block  →

Last updated 4 days ago

Created by

🇩🇪Germany M_Z

Live updates comments and jobs are added and updated live.
  • Needs backport to D7

    After being applied to the 8.x branch, it should be considered for backport to the 7.x branch. Note: This tag should generally remain even after the backport has been written, approved, and committed.

  • Needs issue summary update

    Issue summaries save everyone time if they are kept up-to-date. See Update issue summary task instructions.

  • Novice

    It would make a good project for someone who is new to the Drupal contribution process. It's preferred over Newbie.

Sign in to follow issues

Merge Requests

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

  • Comment over 1 year ago →
    🇮🇳India Nikhil_110

    Attached patch against Drupal 10.1.x

  • Comment over 1 year ago →
    🇮🇳India TanujJain-TJ

    Fixed CCF on #63 and rerolled for Drupal 10.1.x

  • Status changed to Needs review over 1 year ago
  • Comment over 1 year ago →
    🇮🇳India TanujJain-TJ
  • Status changed to Needs work over 1 year ago
  • Comment over 1 year ago →
    🇺🇸United States smustgrave

    Removing credit from both #63 and #64 as #63 didn't check the patch before uploading and removed the fix and some of the tests. #64 carried it forward.

    Also patch #55 still applies to D10.1 so a reroll was not needed.

    #55 tests will have to be updated for D10.1

  • Merge request !6122Issue #2617546: Path comparison (e.g. for block visiblity) doesn't work for aliased internal paths → (Open) created by Hardik_Patel_12
  • Comment 7 months ago →
    🇮🇳India Hardik_Patel_12 India

    An issue summary update is still needed.

  • Comment 7 months ago →
    🇺🇸United States smustgrave

    FYI this issue was tagged for novice for new users. You have 26 pages of posts so definitely not novice. Please try to avoid novice issues in the future

    Thanks!

  • Comment 6 months ago →
    🇩🇪Germany M_Z

    I want to thank everybody who is still working on that issue that I reported more than 8 years ago (for Drupal 7). My initial bug report contained the solution to fix this bug. I also posted a solution to fix this bug for Drupal 8+ more than 4 years ago. I don't think that tests are useless, but this core bug could have been fixed years ago.

    There aren't tests for all PHP lines in the core code. I'm not quite sure if the test for this issue is that essential and relevant. Other "if" statements in core code haven't a corresponding test coverage, too.

    I would suggest to fix this bug in the near future and maybe add a follow-up issue for test coverage. But maybe all the hard-working helpers in this issue are near the finish line to get this solved inclusive test coverage...

contrib.social Blog FAQ Discussions
Production build 0.69.0 2024