? Pending

User tests: Successful: Unsuccessful:

avatar brianteeman
brianteeman
20 May 2021

All uploaded files are saved as lowercase.

Try to upload a file with an uppercase extension eg test.JPG

It fails

Apply PR

Try to upload a file with an uppercase extension eg test.JPG

Success - saved as test.jpg

Pull Request for Issue #33179

avatar brianteeman brianteeman - open - 20 May 2021
avatar brianteeman brianteeman - change - 20 May 2021
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 20 May 2021
Category Administration com_templates
avatar sandramay0905 sandramay0905 - test_item - 21 May 2021 - Tested successfully
avatar sandramay0905
sandramay0905 - comment - 21 May 2021

I have tested this item successfully on 18d57f5


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/34065.

avatar joomdonation
joomdonation - comment - 21 May 2021

From a quick code review, this PR looks wrong to me. It will prevent file such as Test.jpg from being uploaded and it could not be right ?

If I understand the problem right, what is happening right now is the file test.JPG still being uploaded, but it is invisible right now and the right fix would be make it visible ? If so, the right fix would be modify checkFormat method in TemplateModel https://github.com/joomla/joomla-cms/blob/4.0-dev/administrator/components/com_templates/src/Model/TemplateModel.php#L1972 so that file extension is case-insensitive (mean both Test.jpg and Test.JPG will be displayed in template manager).

Modify that method to the following code should work:

protected function checkFormat($ext)
{
		if (!isset($this->allowedFormats))
		{
			$params       = ComponentHelper::getParams('com_templates');
			$imageTypes   = explode(',', $params->get('image_formats'));
			$sourceTypes  = explode(',', $params->get('source_formats'));
			$fontTypes    = explode(',', $params->get('font_formats'));
			$archiveTypes = explode(',', $params->get('compressed_formats'));

			$this->allowedFormats = array_merge($imageTypes, $sourceTypes, $fontTypes, $archiveTypes);
			$this->allowedFormats = array_map('strtolower', $this->allowedFormats);

		}

		return in_array(strtolower($ext), $this->allowedFormats);
}
avatar brianteeman
brianteeman - comment - 21 May 2021

If I understand the problem right, what is happening right now is the file test.JPG still being uploaded, but it is invisible right now and the right fix would be make it visible ?

No. In my tests using testone.JPG and Testtwo.JPG both gave the following error message and were not uploaded

Invalid file name. Please correct the name of the file and upload again.
Failed to upload file.

Happy to admit my code might not be correct but after testing your suggestion the error remained.

With this PR both were uploaded successfully as testone.jpg and testtwo.jpg

However I now see that a file uploaded to the server directly that is mixed case is not displayed in the folder tree - with or without this pr or your suggestion

avatar joomdonation
joomdonation - comment - 21 May 2021

Hmm. I just tried to use today nightly build and file is uploaded
success_uploaded

It is just not being displayed (due to the file extension case) and with the suggested code, it is being displayed. Maybe I mis-understand the issue somehow.

avatar brianteeman
brianteeman - comment - 21 May 2021

I rechecked and still cant upload

The problem with seeing the file in the tree turns out to be super super easy
image

I am going to close this as its obviously not the right way.

avatar brianteeman brianteeman - change - 21 May 2021
Status Pending Closed
Closed_Date 0000-00-00 00:00:00 2021-05-21 09:13:59
Closed_By brianteeman
Labels Added: ?
avatar brianteeman brianteeman - close - 21 May 2021
avatar joomdonation
joomdonation - comment - 21 May 2021

@brianteeman I think I know the reason of the error. You cannot upload file with upercase character because this block of code https://github.com/joomla/joomla-cms/blob/4.0-dev/libraries/src/Filesystem/File.php#L86-L90

Not sure why we decide to make the filename become lowercase in that method like that. Seems a bug to me.

Add a Comment

Login with GitHub to post a comment