New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

com_media: normalizing uploaded file names #23259

Merged
merged 2 commits into from Dec 27, 2018

Conversation

Projects
None yet
6 participants
@infograf768
Copy link
Member

infograf768 commented Dec 10, 2018

Pull Request for Issue #23249

Summary of Changes

Adding hyphens to allowed characters in file names uploaded by drag and drop in TinyMCE.

Normalizing file names in direct com_media upload files to fit with the drag and drop format:

  1. the name of a file should not contain any period . before the extension.
    (real original name of these files are of the type: Screen Shot 2018-12-09 at 07.43.58.png on my Mac)
    This is nicely taken care of when drag and drop as periods are replaced by nothing (after adding the hyphen in the code above).
    Screen_Shot_2018-12-09_at_075108.png
    BUT not in normal upload where I get:
    Screen-Shot-2018-12-10-at-09.29.49.png

  2. Also, spaces are replaced by hyphens in one case and by underscores in the other.

Testing Instructions

Upload a file (usually an image) containing spaces and periods in other place than before the extension, both using com_media and drag and drop.

After patch

An image which name on the desktop is
Screen Shot 2018-12-09 at 07.43.58.png

will be uploaded to Joomla as
Screen_Shot_2018-12-09_at_074358.png

@infograf768 infograf768 changed the title com_media: normalizing file names com_media: normalizing uploaded file names Dec 10, 2018

@@ -85,7 +85,7 @@ public function upload()
$tempExt = (!empty($fileparts['extension'])) ? strtolower($fileparts['extension']) : '';
// Transform filename to punycode, then neglect otherthan non-alphanumeric characters & underscores. Also transform extension to lowercase
$safeFileName = preg_replace(array("/[\\s]/", '/[^a-zA-Z0-9_]/'), array('_', ''), $fileparts['filename']) . '.' . $tempExt;
$safeFileName = preg_replace(array("/[\\s]/", '/[^a-zA-Z0-9_\-]/'), array('_', ''), $fileparts['filename']) . '.' . $tempExt;

This comment has been minimized.

@dgrammatiko

dgrammatiko Dec 10, 2018

Contributor

Can we allow the character @ also? Eg '/[^a-zA-Z0-9_\-@]/' should be ok. Reason: it's quite common for responsive images or images for different pixel density

This comment has been minimized.

@infograf768

infograf768 Dec 11, 2018

Author Member

This would require modifying the library makeSafe() method.

IMHO, this should be considered for another patch.

cs
@Quy

This comment has been minimized.

Copy link
Contributor

Quy commented Dec 20, 2018

I have tested this item successfully on 36ec8ea


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

1 similar comment
@viocassel

This comment has been minimized.

Copy link
Contributor

viocassel commented Dec 24, 2018

I have tested this item successfully on 36ec8ea


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

@infograf768

This comment has been minimized.

Copy link
Member Author

infograf768 commented Dec 24, 2018

Thanks for testing. RTC.


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

@joomla-cms-bot joomla-cms-bot added the RTC label Dec 24, 2018

@infograf768 infograf768 added this to the Joomla 3.9.2 milestone Dec 24, 2018

@mbabker mbabker merged commit f40a32e into joomla:staging Dec 27, 2018

4 checks passed

JTracker/HumanTestResults Human Test Results: 2 Successful 0 Failed.
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/drone/pr the build was successful
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@joomla-cms-bot joomla-cms-bot removed the RTC label Dec 27, 2018

@infograf768 infograf768 deleted the infograf768:media_filename branch Dec 27, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment