Skip to content
Snippets Groups Projects
Verified Commit a335588b authored by Dave Long's avatar Dave Long
Browse files

Issue #2481349 by mfb, dagmar, jofitz, smustgrave, vasi, neclimdul, ziomizar,...

Issue #2481349 by mfb, dagmar, jofitz, smustgrave, vasi, neclimdul, ziomizar, quietone, _utsavsharma, littlepixiez, xjm, fgm, Todd Zebert, hass, dawehner, heddn, webchick, catch: Prevent the use of placeholders that cannot be converted into strings when creating logs

(cherry picked from commit 367e57e7)
parent 285dc9cf
No related branches found
No related tags found
12 merge requests!8376Drupal views: adding more granularity to the ‘use ajax’ functionality,!8300Issue #3443586 View area displays even when parent view has no results.,!7567Issue #3153723 by quietone, Hardik_Patel_12: Change the scaffolding...,!7565Issue #3153723 by quietone, Hardik_Patel_12: Change the scaffolding...,!7509Change label "Block description" to "Block type",!7344Issue #3292350 by O'Briat, KlemenDEV, hswong3i, smustgrave, quietone: Update...,!6922Issue #3412959 by quietone, smustgrave, longwave: Fix 12 'un' words,!6848Issue #3417553 by longwave: Remove withConsecutive() in CacheCollectorTest,!6720Revert "Issue #3358581 by pfrenssen, _tarik_, a.dmitriiev, smustgrave:...,!6560Update ClaroPreRender.php, confirming classes provided are in array format,!6528Issue #3414261 by catch: Add authenticated user umami performance tests,!6501Issue #3263668 by omkar-pd, Wim Leers, hooroomoo: Re-enable inline form errors...
Pipeline #87398 canceled
......@@ -29,9 +29,16 @@ public function parseMessagePlaceholders(&$message, array &$context) {
$key = '@' . $key;
}
}
// To be considered a valid placeholder, the key should be in
// \Drupal\Component\Render\FormattableMarkup style and the variable
// should be a string, number, or \Stringable object. For historical
// reasons, Boolean and NULL placeholders are also allowed; NULL
// placeholders are deprecated and may throw an error in the future.
// @see https://www.drupal.org/node/3318826
if (!empty($key) && ($key[0] === '@' || $key[0] === '%' || $key[0] === ':')) {
// The key is now in \Drupal\Component\Render\FormattableMarkup style.
$variables[$key] = $variable;
if (is_scalar($variable) || is_null($variable) || $variable instanceof \Stringable) {
$variables[$key] = $variable;
}
}
}
......
......@@ -60,6 +60,21 @@ function (callable $hook, string $module) use (&$implementation_count) {
$this->assertEquals(1, $cron_count, "Cron added $cron_count of 1 new log entries");
}
/**
* Tests that only valid placeholders are stored in the variables column.
*/
public function testInvalidPlaceholders() {
\Drupal::logger('my_module')->warning('Hello @string @array @object', ['@string' => '', '@array' => [], '@object' => new \stdClass()]);
$variables = \Drupal::database()
->select('watchdog', 'w')
->fields('w', ['variables'])
->orderBy('wid', 'DESC')
->range(0, 1)
->execute()
->fetchField();
$this->assertSame(serialize(['@string' => '']), $variables);
}
/**
* Runs cron and returns number of new log entries.
*
......
......@@ -4,6 +4,7 @@
namespace Drupal\Tests\Core\Logger;
use Drupal\Component\Render\FormattableMarkup;
use Drupal\Core\Logger\LogMessageParser;
use Drupal\Tests\UnitTestCase;
......@@ -40,38 +41,62 @@ public function testParseMessagePlaceholders(array $value, array $expected) {
*/
public function providerTestParseMessagePlaceholders() {
return [
// PSR3 only message.
[
'PSR3-style placeholder' => [
['message' => 'User {username} created', 'context' => ['username' => 'Dries']],
['message' => 'User @username created', 'context' => ['@username' => 'Dries']],
],
// PSR3 style mixed in a format_string style message.
[
'PSR3- and FormattableMarkup-style placeholders' => [
['message' => 'User {username} created @time', 'context' => ['username' => 'Dries', '@time' => 'now']],
['message' => 'User @username created @time', 'context' => ['@username' => 'Dries', '@time' => 'now']],
],
// format_string style message only.
[
'FormattableMarkup-style placeholder' => [
['message' => 'User @username created', 'context' => ['@username' => 'Dries']],
['message' => 'User @username created', 'context' => ['@username' => 'Dries']],
],
// Message without placeholders but wildcard characters.
[
'Wildcard characters' => [
['message' => 'User W-\\};~{&! created @', 'context' => ['' => '']],
['message' => 'User W-\\};~{&! created @', 'context' => []],
],
// Message with double PSR3 style messages.
[
'Multiple PSR3-style placeholders' => [
['message' => 'Test {with} two {{encapsuled}} strings', 'context' => ['with' => 'together', 'encapsuled' => 'awesome']],
['message' => 'Test @with two {@encapsuled} strings', 'context' => ['@with' => 'together', '@encapsuled' => 'awesome']],
],
// Test removal of unexpected placeholders like ! while allowed
// placeholders beginning with @, % and : are preserved.
[
'Disallowed placeholder' => [
['message' => 'Test placeholder with :url and old !bang parameter', 'context' => [':url' => 'https://example.com', '!bang' => 'foo bar']],
['message' => 'Test placeholder with :url and old !bang parameter', 'context' => [':url' => 'https://example.com']],
],
'Stringable object placeholder' => [
['message' => 'object @b', 'context' => ['@b' => new FormattableMarkup('convertible', [])]],
['message' => 'object @b', 'context' => ['@b' => 'convertible']],
],
'Non-placeholder context value' => [
['message' => 'message', 'context' => ['not_a_placeholder' => new \stdClass()]],
['message' => 'message', 'context' => []],
],
'Non-stringable array placeholder' => [
['message' => 'array @a', 'context' => ['@a' => []]],
['message' => 'array @a', 'context' => []],
],
'Non-stringable object placeholder' => [
['message' => 'object @b', 'context' => ['@b' => new \stdClass()]],
['message' => 'object @b', 'context' => []],
],
'Non-stringable closure placeholder' => [
['message' => 'closure @c', 'context' => ['@c' => function () {}]],
['message' => 'closure @c', 'context' => []],
],
'Non-stringable resource placeholder' => [
['message' => 'resource @r', 'context' => ['@r' => fopen('php://memory', 'r+')]],
['message' => 'resource @r', 'context' => []],
],
'Non-stringable placeholder is not the first placeholder' => [
['message' => 'mixed @a @b @c', 'context' => ['@a' => 123, '@b' => [1], '@c' => TRUE]],
['message' => 'mixed @a @b @c', 'context' => ['@a' => 123, '@c' => TRUE]],
],
'NULL and Boolean placeholders are considered stringable' => [
['message' => 'mixed @a @b @c', 'context' => ['@a' => NULL, '@b' => TRUE, '@c' => FALSE]],
['message' => 'mixed @a @b @c', 'context' => ['@a' => NULL, '@b' => TRUE, '@c' => FALSE]],
],
];
}
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment