Follow-up to #2570285: Make sure TranslatableMarkup accepts string values only

Follow-up to #2557113: Make t() return a TranslationWrapper object to remove reliance on a static, unpredictable safe list

Problem/Motivation

t() accepts non-strings, empty strings and placeholders without further text such as t('@placeholder', ['@placeholder' => $variable], which can cause various issues.

Proposed solution

Fail when input doesn't make sense

CommentFileSizeAuthor
#30 2571631-nr-bot.txt150 bytesneeds-review-queue-bot
#3 ensure_no-2571631-3.patch7.97 KBcpj
#2 ensure_no-2571631-2.patch3.96 KBcpj

Issue fork drupal-2571631

Command icon 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:

About issue forks
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

cpj created an issue. See original summary.

cpj’s picture

cpj CreditAttribution: cpj commented
FileSize
ensure_no-2571631-2.patch3.96 KB

First patch to start looking for the errors

cpj’s picture

cpj CreditAttribution: cpj commented
FileSize
ensure_no-2571631-3.patch7.97 KB

These are the proposed corrections to the no-placeholder strings, identified by the @alexpott regex t\('[@%!]\w+[\W']*'

The last submitted patch, 2: ensure_no-2571631-2.patch, failed testing.

Status: Needs review » Needs work

The last submitted patch, 3: ensure_no-2571631-3.patch, failed testing.

Eric_A’s picture

Eric_A CreditAttribution: Eric_A commented

I just took a quick look at #3 and noticed that t('@zone', array('@zone' => t(str_replace('_', ' ', $zone)))); was missing.

In #1854510-6: Remove date formatting from system_time_zones() I provided a patch for this one.

The last submitted patch, 2: ensure_no-2571631-2.patch, failed testing.

The last submitted patch, 3: ensure_no-2571631-3.patch, failed testing.

plach’s picture

plach
he/him
Primary language Italian
Location Venezia
CreditAttribution: plach as a volunteer commented
Priority: Major » Normal

From the issue summary this does not look major, but feel free to revert if I'm missing something. Please update the IS if you do...

Version: 8.0.x-dev » 8.1.x-dev

Drupal 8.0.6 was released on April 6 and is the final bugfix release for the Drupal 8.0.x series. Drupal 8.0.x will not receive any further development aside from security fixes. Drupal 8.1.0-rc1 is now available and sites should prepare to update to 8.1.0.

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

Version: 8.1.x-dev » 8.2.x-dev

Drupal 8.1.9 was released on September 7 and is the final bugfix release for the Drupal 8.1.x series. Drupal 8.1.x will not receive any further development aside from security fixes. Drupal 8.2.0-rc1 is now available and sites should prepare to upgrade to 8.2.0.

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

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.6 was released on February 1, 2017 and is the final full bugfix release for the Drupal 8.2.x series. Drupal 8.2.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.3.0 on April 5, 2017. (Drupal 8.3.0-alpha1 is available for testing.)

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

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.6 was released on August 2, 2017 and is the final full bugfix release for the Drupal 8.3.x series. Drupal 8.3.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.4.0 on October 4, 2017. (Drupal 8.4.0-alpha1 is available for testing.)

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

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.4 was released on January 3, 2018 and is the final full bugfix release for the Drupal 8.4.x series. Drupal 8.4.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.5.0 on March 7, 2018. (Drupal 8.5.0-alpha1 is available for testing.)

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

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.6 was released on August 1, 2018 and is the final bugfix release for the Drupal 8.5.x series. Drupal 8.5.x will not receive any further development aside from security fixes. Sites should prepare to update to 8.6.0 on September 5, 2018. (Drupal 8.6.0-rc1 is available for testing.)

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

Version: 8.6.x-dev » 8.8.x-dev

Drupal 8.6.x will not receive any further development aside from security fixes. Bug reports should be targeted against the 8.8.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.9.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.

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.

quietone’s picture

quietone CreditAttribution: quietone as a volunteer commented
Assigned: cpj » Unassigned
Status: Needs work » Closed (outdated)
Issue tags: +Bug Smash Initiative

This was fixed in #2570285: Make sure TranslatableMarkup accepts string values only.

Therefore, closing as outdated. If this is incorrect reopen the issue, by setting the status to 'Active', and add a comment explaining what still needs to be done.

Thanks!

Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir at MD Systems GmbH commented
Status: Closed (outdated) » Needs work

This is not the same issue, it was actually a follow-up of that.

That issue is about non-strings.

This is about strings that only contain a placeholder and nothing else, see the changes in the patch. It's still a string, it's just not something that makes sense to translate.

quietone’s picture

quietone CreditAttribution: quietone as a volunteer commented

@Berdir, thanks for fixing my error. I really didn't read the IS correctly.

apaderno’s picture

apaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
CreditAttribution: apaderno as a volunteer commented
Version: 8.9.x-dev » 9.2.x-dev

On Drupal 8.9.x, only security issues are fixed.

mohit_aghera’s picture

mohit_aghera CreditAttribution: mohit_aghera at QED42 commented
Version: 9.2.x-dev » 9.3.x-dev

mohit_aghera opened merge request !1173

mohit_aghera’s picture

mohit_aghera CreditAttribution: mohit_aghera at QED42 commented
Status: Needs work » Needs review

I've used following regex to find the occurances:
t\('[@%!]\w+[\W']*'

@Berdir:

In patch #3, there is additional validation to ensure non-empty string.
Do we need to consider that as well?

During the search, I came across one string-related issue.
system.admin.inc line 143
$form[$dependency]['#required_by'][] = $theme->status ? t('@theme', ['@theme (theme)' => $theme->info['name']]) : t('@theme (theme) (<span class="admin-disabled">disabled</span>)', ['@theme' => $theme->info['name']]);

The text for the TRUE condition in the above ternary operation seems to have some issues. t('@theme', ['@theme (theme)' => $theme->info['name']])
I've updated it something like t('@theme (theme)', ['@theme' => $theme->info['name']]) to prevent to breaks.

Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir at MD Systems GmbH commented
Status: Needs review » Needs work

Reviewed.

mohit_aghera’s picture

mohit_aghera CreditAttribution: mohit_aghera at QED42 commented
Status: Needs work » Needs review

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.0-rc1 was released on November 26, 2021, which means new developments and disruptive changes should now be targeted for the 9.4.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.0-alpha1 was released on May 6, 2022, which means new developments and disruptive changes should now 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.

Version: 9.5.x-dev » 10.1.x-dev

Drupal 9.5.0-beta2 and Drupal 10.0.0-beta2 were released on September 29, 2022, which means new developments and disruptive changes should now 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.

needs-review-queue-bot’s picture

needs-review-queue-bot CreditAttribution: needs-review-queue-bot as a volunteer commented
Status: Needs review » Needs work
FileSize
2571631-nr-bot.txt150 bytes

The Needs Review Queue Bot tested this issue. It either no longer applies to Drupal core, or fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

Apart from a re-roll or rebase, this issue may need more work to address feedback in the issue or MR comments. To progress an issue, incorporate this feedback as part of the process of updating the issue. This helps other contributors to know what is outstanding.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

Version: 10.1.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, which currently accepts only minor-version allowed changes. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.