AI generated bug and resolution disclaimer

This bug was discovered and a patch resolution was suggested by Claude Sonnet 5 using a custom agent trained on Drupal module and theme development.

Problem/Motivation

After setting a webform's purge settings to `purge: all` and `purge_days: 365` (or some other number of days to keep webform submissions around) and then running `drush cron`, these errors are thrown, which erroneously show the `bf://` protocol trying to be used to load webform submissions that are needing to be purged:

```
Could not load file data for bf://webform/contact_2/251706
Could not load file data for bf://webform/contact_2/251751
Could not load file data for bf://webform/contact_2/251766
Could not load file data for bf://webform/contact_2/251781
Could not load file data for bf://webform/contact_2/251816
Could not load file data for bf://webform/contact_2/251826
Could not load file data for bf://webform/contact_2/251841
Could not load file data for bf://webform/contact_2/251906
Could not load file data for bf://webform/contact_2/251911
Could not load file data for bf://webform/contact_2/251916
```

This happenes because BrandfolderStreamWrapper::url_stat() logs a watchdog "error" any time loadFileData() fails to resolve a `bf://` URL, without checking whether the URL was ever meant to be a Brandfolder attachment/asset reference in the first place:

    public function url_stat($url, $flags): array {
      $file_data_loaded = $this->loadFileData($url);
      if (!$file_data_loaded) {
        $this->logger->error('Could not load file data for :url', [':url' => $url]);
      }
      return $this->stream_stat();
    }

loadFileData() delegates to brandfolder_parse_uri(), whose regex only matches genuine Brandfolder URIs shaped like `bf://{cdn_id}/(as|at)/{id}/{filename}`. Any other module that registers its own directory convention and happens to call is_dir()/file_exists() against a `bf://...` path (because `bf` is registered as a WRITE_VISIBLE stream wrapper) will always fail to parse, and this module logs it as an "error" every single time - even though "the path doesn't exist" is a completely normal, silent outcome for every other stream wrapper (public://, private://, etc).

Steps to reproduce

This surfaced in production via Webform module's submission purge, but it can be reproduced directly:

1. Enable the Webform module and set `purge: all` / `purge_days` > 0 on any webform (no file/managed_file/signature elements are required).
2. Run cron (or `drush cron`) once enough submissions are old enough to be purged.
3. Observe repeated dblog/watchdog "brandfolder" channel errors like:

> Could not load file data for bf://webform/{webform_id}/{sid}

This happens because WebformSubmissionStorage::delete() loops over *every* registered write-visible stream wrapper scheme (not just ones actually used by the webform) to clean up an empty per-submission upload directory:

       $file_directory = $stream_wrapper . '://webform/' . $webform->id() . '/' . $entity->id();
       if (is_dir($file_directory) && empty($this->fileSystem->scanDirectory($file_directory, '/.*/'))) {
         $this->fileSystem->deleteRecursive($file_directory);
       }

Since `bf` is registered as WRITE_VISIBLE (see brandfolder.services.yml: `{ name: stream_wrapper, scheme: bf }`), is_dir() is called against a `bf://webform/...` path that was never a real Brandfolder file, triggering url_stat() -> loadFileData() -> brandfolder_parse_uri() failure -> logged error.

You can also reproduce the parse failure directly, without Webform:

       drush php:eval "var_dump(brandfolder_parse_uri('bf://webform/contact_2/5756'));"
       // => bool(false)

Proposed resolution

Only log an error when the URL actually parses as a Brandfolder attachment/asset reference (i.e. brandfolder_parse_uri() returns a non-empty result with an id) but the underlying data lookup still failed - that's a genuine orphaned-reference problem worth logging. URLs that were never shaped like a Brandfolder reference to begin with should fail silently, consistent with how every other stream wrapper handles is_dir()/stat() on a nonexistent path.

Patch attached implementing this in src/StreamWrapper/BrandfolderStreamWrapper.php.

This is log noise only - no data is lost or corrupted by the current behavior, since is_dir() still correctly evaluates to false either way and purge/cleanup completes normally. But the error-level noise is misleading and pollutes logs at scale (any bulk process that touches many nonexistent bf:// paths, like a submission purge across 50 webforms, produces one log entry per item).

Remaining tasks

- Review/test patch.
- Consider whether other stream wrapper methods (e.g. stream_open) have the same overly-broad error logging pattern.

I'm having some issues uploading the patch, so I'll drop the code change here and then create a follow-up MR later.

```
--- a/src/StreamWrapper/BrandfolderStreamWrapper.php
+++ b/src/StreamWrapper/BrandfolderStreamWrapper.php
@@ -226,7 +226,14 @@
// Load file size from DB before proceeding.
$file_data_loaded = $this->loadFileData($url);
if (!$file_data_loaded) {
- $this->logger->error('Could not load file data for :url', [':url' => $url]);
+ // Only log when the URL is actually shaped like a Brandfolder
+ // attachment/asset reference; other callers (e.g. Webform's per-
+ // submission upload directory cleanup) probe every registered
+ // stream wrapper scheme and should fail silently like public/private.
+ $uri_parts = brandfolder_parse_uri($url);
+ if (!empty($uri_parts) && !empty($uri_parts['id'])) {
+ $this->logger->error('Could not load file data for :url', [':url' => $url]);
+ }
}

return $this->stream_stat();
```

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

wesleymusgrove created an issue. See original summary.

wesleymusgrove’s picture

Issue summary: View changes
StatusFileSize
new901 bytes