Skip to content
Snippets Groups Projects

Issue #3375447: FileUploadHandler::handleFileUpload() assumes an UploadedFile object

Closed Kim Pepper requested to merge issue/drupal-3375447:3375447-uploadedfile-constraint into 11.x

Merge request reports

Merge request pipeline passed for cfbe3ae2

Code Quality is loading

Closed by Lee RowlandsLee Rowlands 1 year ago (Oct 26, 2023 4:36am UTC)

Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
  • Lee Rowlands
  • Lee Rowlands
  • Lee Rowlands
    • Resolved by Kim Pepper

      Looks good, I think there's one BC concern we're missing here.

      Previously ::handleFileUpload used exceptions to control flow in the invalid cases, but now it does not (by design).

      However, calling code might be relying on those exceptions to check if the result is valid, and as such won't know it needs to call $result->getViolations()> 0 (or a new ::isValid as discussed above).

      I think we need an additional parameter to ::handleFileUpload that is bool $throw which defaults to TRUE. In new code we can pass FALSE.

      In old code, it will get the default of TRUE.

      If $throw is TRUE, we should check the violation list and throw an Exception if the file isn't valid.

      Additionally, when $throw is true we can also call trigger_error to trigger a deprecation error to communicate that the calling code needs to update to use ::isValid or similar.

  • Kim Pepper added 6 commits

    added 6 commits

    Compare with previous version

  • Kim Pepper added 1 commit

    added 1 commit

    • c55757d9 - Add deprecation triggers and bc support for throws

    Compare with previous version

  • Kim Pepper added 1 commit

    added 1 commit

    • 38ef43d4 - Change signature of validate to only take options

    Compare with previous version

  • Kim Pepper resolved all threads

    resolved all threads

  • Kim Pepper added 1 commit

    added 1 commit

    • 4ced0fa1 - Adds legacy file upload handler test

    Compare with previous version

  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Please register or sign in to reply
    Loading