FileUploadHandler::handleExtensionValidation() should check for empty extensions string

Created on 20 December 2023, 6 months ago
Updated 2 March 2024, 4 months ago

Problem/Motivation

Currently, logic for deciding whether to remove extension validation checks:

  if (isset($validators['FileExtension'])) {
      if (!isset($validators['FileExtension']['extensions'])) {
        // If 'FileExtension' is set and the list is empty then the caller wants
        // to allow any extension. In this case we have to remove the validator
        // or else it will reject all extensions.
        unset($validators['FileExtension']);
      }
    }

However, the code comments say:

// If 'FileExtension' is set and the list is empty then the caller wants
// to allow any extension. In this case we have to remove the validator
// or else it will reject all extensions.

Either the code is wrong or the comment is wrong.

There are tests that rely on having an empty extensions list to not trigger the security rename feature later in the code.

We need to evaluate whether the test assumptions are correct.

Steps to reproduce

Proposed resolution

Change the code to check for empty instead:

  if (isset($validators['FileExtension'])) {
      if (empty($validators['FileExtension']['extensions'])) {
        // If 'FileExtension' is set and the list is empty then the caller wants
        // to allow any extension. In this case we have to remove the validator
        // or else it will reject all extensions.
        unset($validators['FileExtension']);
      }
    }

Remaining tasks

Fix test failures.

User interface changes

API changes

Data model changes

Release notes snippet

๐Ÿ› Bug report
Status

Closed: won't fix

Version

11.0 ๐Ÿ”ฅ

Component
File moduleย  โ†’

Last updated 3 days ago

Created by

๐Ÿ‡ฆ๐Ÿ‡บAustralia kim.pepper ๐Ÿ„โ€โ™‚๏ธ๐Ÿ‡ฆ๐Ÿ‡บSydney, Australia

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Merge Requests

Comments & Activities

Production build 0.69.0 2024