Correct url placeholder in update.inc

Created on 7 November 2022, about 2 years ago
Updated 8 August 2024, 5 months ago

Problem/Motivation

When performing database updates on a site that has an entry in the system.schema key/value storage for a module that is missing from the site, Drupal returns a warning and creates a db log entry saying "Module [module_name_appears_here] has an entry in the system.schema key/value storage, but is missing from your site. More information about this error." -- and the "More information about this error" sentence is wrapped in an tag with an improper URL, e.g. "http://[base_url]/admin/reports/dblog/event/:url".

Steps to reproduce

In a Drupal 9 website, remove a module without uninstalling it, then perform database updates.

Proposed resolution

In declaring the arguments to pass to the translation function in update.inc, change the URL placeholder from ":url" to "%url", following the treatment of the module name.

Remaining tasks

Patch forthcoming.

๐Ÿ› Bug report
Status

Closed: cannot reproduce

Version

11.0 ๐Ÿ”ฅ

Component
Database updateย  โ†’

Last updated 2 days ago

No maintainer
Created by

๐Ÿ‡บ๐Ÿ‡ธUnited States COBadger

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Comments & Activities

Not all content is available!

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

  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia mehul.gada Mumbai

    mehul.gada โ†’ made their first commit to this issueโ€™s fork.

  • @mehulgada opened merge request.
  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia mehul.gada Mumbai

    Hi @COBadger,

    I guess we don't need to replace URL placeholder from ":url" to "%url". If we use "%variable", it is also getting processed by placeholderEscape() and that adds "" tag around the placeholder variable. For replacing URL, we should specifically use ":variable", so that it is filtered for harmful URL protocol and at that same time "" tag is not added. In our case, the URL variable is used within the anchor tag in the warning text and if we use "%url", it is getting surrounded by "" tag, causing the anchor tag HTML to break (see attached screenshot) and accordingly breaking the unit test. So, I guess we should close this issue without fixing it.

    Ref - https://chromatichq.com/insights/drupal-code-standards-t-function/

  • Status changed to Needs review almost 2 years ago
  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia mehul.gada Mumbai
  • Status changed to Needs work over 1 year ago
  • ๐Ÿ‡บ๐Ÿ‡ธUnited States smustgrave

    MR has a failure.

  • Status changed to Postponed: needs info over 1 year ago
  • ๐Ÿ‡ณ๐Ÿ‡ฟNew Zealand quietone

    I tested this on Drupal 10.0.0, standard install and was not able to reproduce the problem. The message in question is generated in to different places in update.inc. I forced that code to execute during an update (via a temporary update hook). And in both cases the message displayed on screen and in the logs had the correct URL. And just to be sure, I clicked them and they worked.

    Still trying to find the error I used git log -L to review the history of those lines. I did not find any changes to them since the code was originally committed.

    And it is using the correct placeholder, isn't it?

    :variable: Return value is escaped with \Drupal\Component\Utility\Html::escape() and filtered for dangerous protocols using UrlHelper::stripDangerousProtocols(). Use this when using the "href" attribute, ensuring the attribute value is always wrapped in quotes: 
    

    from https://api.drupal.org/api/drupal/core%21lib%21Drupal%21Component%21Rend...

    Can anyone confirm the problem exists on a currently supported version of Drupal?

  • Status changed to Closed: cannot reproduce 5 months ago
  • ๐Ÿ‡ฆ๐Ÿ‡บAustralia acbramley

    Thanks for reporting this issue. We rely on issue reports like this one to resolve bugs and improve Drupal core.

    As part of the Bug Smash Initiative, we are triaging issues that are marked "Postponed (maintainer needs more info)". This issue was marked "Postponed (maintainer needs more info)" more than 1 year ago.

    Since we need more information to move forward with this issue, I am closing it.

    Please feel free to reopen with more information.

Production build 0.71.5 2024