Problem/Motivation
The file validation logic changes in #3409599: Drupal\Component\Plugin\Exception\PluginNotFoundException: The "webform_file_validate_extensions" plugin does not exist broke backwards compatibility.
Prior to this change, any item defined in #upload_validators would be checked to see if it was a callback function, after that logic finished all of the values were passed through to hook_file_validate().
With this change, any item defined in #upload_validators is first checked to see if it is a callback, if it is not it is assumed to be a constraint plugin and it is passed to $this->constraintManager->create($validator, $options) inside \Drupal\file\Validation\FileValidator::validate(). This has caused problems with Webform, IMCE, and possibly others, along with possible custom file uploaders.
Steps to reproduce
Add arbitrary data to the '#upload_validators' form array key. A PluginNotFoundException will be thrown.
Proposed resolution
Catch PluginNotFoundException and trigger a deprecation warning that only valid plugin names will be allowed in the '#upload_validators' form array in Drupal 11.
Remaining tasks
Update change record as per #11
User interface changes
API changes
Data model changes
Release notes snippet
Issue fork drupal-3410126
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:
- 3410126-file-validation-logic
changes, plain diff MR !5908
Comments
Comment #2
larowlanI think the solution should be that we catch the plugin not found and trigger a deprecation error and carry on.
Webform isn't using it for file validation, its using it for theming.
It probably should use something else, but yes, this is a change in behaviour
Comment #3
larowlanThe code from 10.1
As you can see it silently ignored non existent functions
Comment #4
larowlanComment #7
kim.pepperComment #8
damienmckennaIs there a cleaner way of doing it than a try/catch block?
Comment #9
smustgrave commentedFor the open thread to add a trigger_error, not sure if we can reuse an existing CR since this was covered somewhere else?
Comment #10
kim.pepperComment #11
kim.pepperI think we can add something about passing arbitrary data in the form array under
'#upload_validators'is deprecated to the existing CR https://www.drupal.org/node/3363700. As that CR is published, do we need to do it after this is committed?Comment #12
smustgrave commentedJust for consistency can the issue summary be updated to include steps, proposed solution, etc.
Comment #13
kim.pepperUpdated the issue summary.
Comment #14
catchI think given the thing we're deprecating here already doesn't work in 10.2.x, it's fine to update the CR before commit here.
Comment #15
catchUpdated https://www.drupal.org/node/3363700
Comment #16
alexpottI think the deprecation message needs to mention the string being used as a plugin ID so that this is easier to debug.
Comment #17
catchApplied both those suggestions.
Comment #18
larowlanComment #19
smustgrave commentedAppears feedback has been addressed.
Since this is a regression assumed it's fine that deprecation is 10.2
Comment #20
larowlanIssue credits
Comment #23
larowlanCommitted to 11.x and backported to 10.2.x
Verified the CR changes https://www.drupal.org/node/3363700/revisions/view/13228435/13360530
Thanks all