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();
```
| Comment | File | Size | Author |
|---|---|---|---|
| #2 | brandfolder-suppress-non-attachment-url-stat-log-noise.patch | 901 bytes | wesleymusgrove |
Issue fork brandfolder-3615062
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
Comment #2
wesleymusgrove commented