As it's documented on https://api.drupal.org/api/drupal/includes%21bootstrap.inc/function/watc... the third parameter is an array or have to be set to NULL if you want to make the message untranslatable.
Keeping the backward compatibility possible we with @alexverb propose the following change in dblog module in order to be able to handle other types too (see patch), because currently if you use e.g. a string as third parameter, watchdog writes the record to db, and it brakes the interface until you don't remove the wrong item from db. Also to make it less possible for contributed and custom solutions to do the same mistake, it would be good if even a wrong value goes to watchdog db table, it won't crash the interface just because theme_dblog_message function doesn't handle it well.

As 'N;' string is the manifestum of an empty array after serialization, it makes confusion why it has to be identically equal when it gets compared with the variables. Instead of this wizard it should be checked is it an array or not after unserializing it.

Following the documentation of watchdog, we think with this solution it's more realistic and understandable how theme_dblog_message works.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

tatarbj created an issue. See original summary.

tatarbj’s picture

tatarbj’s picture

Assigned: tatarbj » Unassigned
Status: Active » Needs review
Wim Leers’s picture

Component: dynamic_page_cache.module » dblog.module
dagmar’s picture

Version: 7.54 » 7.x-dev
Status: Needs review » Needs work
Issue tags: -watchdog

This is already fixed in D8, but with some minor modifications:

  public function formatMessage($row) {
    // Check for required properties.
    if (isset($row->message, $row->variables)) {
      $variables = @unserialize($row->variables);
      // Messages without variables or user specified text.
      if ($variables === NULL) {
        $message = Xss::filterAdmin($row->message);
      }
      elseif (!is_array($variables)) {
        $message = $this->t('Log data is corrupted and cannot be unserialized: @message', ['@message' => Xss::filterAdmin($row->message)]);
      }
      // Message to translate with injected variables.
      else {
        $message = $this->t(Xss::filterAdmin($row->message), $variables);
      }
    }
    else {
      $message = FALSE;
    }
    return $message;
  }

It would be nice to see this implemented in the same way in D7.

Wim Leers’s picture

Title: Watchdog brakes when its third parameter is a string » Watchdog breaks when its third parameter is a string
lisotton’s picture

Assigned: Unassigned » lisotton

Working on a new patch.

lisotton’s picture

Status: Needs work » Needs review
FileSize
1.17 KB
1.1 KB

New patch using the same approach of D8.

mgoncalves’s picture

Assigned: lisotton » mgoncalves

Reviewing it.

code-drupal’s picture

The line in the patch submitted seems incorrect.
$args = @unserialize($event->variables)

If the unserialize() statement is preceded with a '@' to avoid cluttering the logs with warns or notices there will be absolutely no clue as to why the script stopped working.

lisotton’s picture

Hi @code-drupal,

It's the same approach used in Drupal 8 as suggested by @dagmar.

Vj’s picture

FileSize
121.11 KB
140.75 KB

Tested patch #8. Worked for me.

Screenshot before patch
Before patch

Screenshot after patch
After patch

mgoncalves’s picture

Assigned: mgoncalves » Unassigned
Status: Needs review » Reviewed & tested by the community
FileSize
35.22 KB
31.57 KB

Hello all,

This is result of patch #8.

Before:
Error

After:
Success

Works for me.

Thanks for the patch @lisotton.

David_Rothstein’s picture

Seems closely related to #1279680: watchdog() does not type its array arguments - I think we want to fix it at the root (as done in some of the patches there) but fixing it here also might be OK (especially for sites that already have broken entries in the logs).

David_Rothstein’s picture

Status: Reviewed & tested by the community » Closed (duplicate)

This actually looks like a duplicate of #2790857: Log completely unusable when an entry has corrupt serialized data (D7) which is further along than this since it's backporting the tests from Drupal 8 also, but the patch here doesn't have any tests. Let's consolidate efforts to fix this over there.