Project

General

Profile

Actions

Bug #65673

closed

"0" is not accepted as a valid file name

Added by Malte Muth about 9 years ago. Updated over 5 years ago.

Status:
Closed
Priority:
Should have
Assignee:
-
Category:
File Abstraction Layer (FAL)
Target version:
Start date:
2015-03-11
Due date:
% Done:

100%

Estimated time:
TYPO3 Version:
6.2
PHP Version:
5.5
Tags:
Complexity:
no-brainer
Is Regression:
No
Sprint Focus:

Description

After sanitizing a filename in LocalDriver, the result $cleanFilename is checked for ... any surviving characters, I guess?

Line 320-325, typo3/sysext/core/Classes/Resource/Driver/LocalDriver.php:

if (!$cleanFileName) {
    throw new \TYPO3\CMS\Core\Resource\Exception\InvalidFileNameException(
        'File name ' . $cleanFileName . ' is invalid.',
        1320288991
    );
}

The purpose of the error message is not quite clear; either $cleanFileName is empty since the file name did not contain any valid characters to begin with, or it is NULL as a return value of preg_replace when the function encountered an error.

The check, however, catches any file name that evaluates to false after coercion (although the file name is perfectly valid).

Without any deeper understanding, I propose to change the condition to

if (is_null($cleanFileName) || empty($cleanFileName)) {

Discuss!

Actions #1

Updated by Malte Muth about 9 years ago

Actually, after reading up how empty works, the second condition should be

0 == strlen($cleanFileName)
Actions #2

Updated by Markus Klein about 9 years ago

  • Category set to File Abstraction Layer (FAL)
  • Status changed from New to Accepted
  • Target version set to next-patchlevel

you shouldn't use strlen() to check for empty strings, a straight comparision === '' is much faster. ;-)

Wanna push a patch for that?

Actions #3

Updated by Gerrit Code Review about 9 years ago

  • Status changed from Accepted to Under Review

Patch set 1 for branch TYPO3_6-2 of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at http://review.typo3.org/37768

Actions #4

Updated by Malte Muth about 9 years ago

Hi Markus,

thanks for the hint. I'm new to the gerrit process, can you let me know if what I've done is correct?

Also: I think the error message should be changed to include the unsanitized filename in some way. Right now, $cleanFileName is not display-able anyway.

Actions #5

Updated by Gerrit Code Review about 9 years ago

Patch set 1 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at http://review.typo3.org/37782

Actions #6

Updated by Gerrit Code Review about 9 years ago

Patch set 2 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at http://review.typo3.org/37782

Actions #7

Updated by Gerrit Code Review about 9 years ago

Patch set 2 for branch TYPO3_6-2 of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at http://review.typo3.org/37768

Actions #8

Updated by Malte Muth about 9 years ago

  • Status changed from Under Review to Resolved
  • % Done changed from 0 to 100
Actions #9

Updated by Gerrit Code Review about 9 years ago

  • Status changed from Resolved to Under Review

Patch set 3 for branch TYPO3_6-2 of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at http://review.typo3.org/37768

Actions #10

Updated by Malte Muth about 9 years ago

  • Status changed from Under Review to Resolved
Actions #11

Updated by Benni Mack over 5 years ago

  • Status changed from Resolved to Closed
Actions

Also available in: Atom PDF