Problem/Motivation

In file_tokens() the case 'owner': handler dereferences $file->getOwner() without a null check:

  case 'owner':
    $owner = $file->getOwner();
    $bubbleable_metadata->addCacheableDependency($owner);
    $name = $owner->label();           // ← fatal if $owner is NULL
    $replacements[$original] = $name;
    break;

EntityOwnerInterface::getOwner() returns NULL when the file's uid references a user that no longer exists (e.g., the owning account was deleted without reassigning the file's content). The chained-token block a few lines below (file.module:842) already guards for this:

  if (($owner_tokens = $token_service->findWithPrefix($tokens, 'owner')) && $file->getOwner()) {

…but the simple [file:owner] token does not, so any token tree render or replacement that walks into a file with an orphaned uid produces:

Error: Call to a member function label() on null in file_tokens() (line 828 of core/modules/file/file.module).

The same defect exists in 11.x at core/modules/file/src/Hook/TokenHooks.php:90-95.

Steps to reproduce

  1. Install Drupal 10.6.x with file, plus devel and token (contrib).
  2. Upload a managed file as user A, then delete user A and choose "Delete the account and its content"… but allow some referencing entity (e.g., a file field on a custom entity) to retain the file row in file_managed. In a long-lived site this state arises naturally over time.
  3. Visit /devel/token// for any entity type that exposes a file-bearing field (in our case competency/9).
  4. Result: WSOD with the trace above.

Direct reproducer:

  $file = File::load($orphan_fid); // a file whose uid points to a deleted user
  \Drupal::token()->replace('[file:owner]', ['file' => $file]);

Proposed resolution

Mirror the existing [file:owner:*] null check on the simple [file:owner] token. Patch attached.

  case 'owner':
    $owner = $file->getOwner();
    if ($owner !== NULL) {
      $bubbleable_metadata->addCacheableDependency($owner);
      $replacements[$original] = $owner->label();
    }
    break;

Apply in both:

  • core/modules/file/file.module (10.x)
  • core/modules/file/src/Hook/TokenHooks.php (11.x)

Remaining tasks

  • Review.
  • Add kernel-test coverage for [file:owner] against a file with a deleted owner (and the chained [file:owner:name] for parity).
  • Decide whether to add a defensive user entity-type cache tag when the owner is missing so the replacement re-evaluates if a user with that uid is later created.

User interface changes

None — the token is simply not replaced, matching current [file:owner:*] behavior.

Issue fork drupal-3588332

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:

Comments

benstallings created an issue. See original summary.

cilefen’s picture

Version: 10.6.x-dev » main

This should be the main branch unless the bug only exists in 10.6.

benstallings’s picture

Thanks, @cilefen, I was getting there, I needed to fix it for the version I'm actually running first. I had assigned to myself so other people wouldn't change it while I was working on it! :)

benstallings’s picture

If you can refrain for a few more minutes, I'll unassign myself when I'm done! Thanks!

benstallings’s picture

Assigned: benstallings » Unassigned
Status: Active » Needs review

I should mention that the user who found this bug was @mattlibby. I don't seem to have the ability to give him credit in the contribution record.

cilefen’s picture

Sorry about that!

smustgrave’s picture

Status: Needs review » Needs work
Issue tags: +Needs tests

Can we get a test case showing this please. Kinda wonder if $replacements[$original] should still get some value, if that being empty could cause issues in other places.

Thank you

benstallings’s picture

Assigned: Unassigned » benstallings
benstallings’s picture

Assigned: benstallings » Unassigned
Status: Needs work » Needs review

@smustgrave thank you for the feedback. I changed the D11 branch to use user 0 as the default value and added a test.

smustgrave’s picture

Status: Needs review » Needs work

Left a comment on the MR.

benstallings’s picture

Status: Needs work » Needs review