Skip to content
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

MODX 3.0.0 Media Browser Upload strips the . from the file name #16144

Closed
PeterBcreates opened this issue Apr 14, 2022 · 22 comments
Closed

MODX 3.0.0 Media Browser Upload strips the . from the file name #16144

PeterBcreates opened this issue Apr 14, 2022 · 22 comments
Labels
bug The issue in the code or project, which should be addressed.

Comments

@PeterBcreates
Copy link

Bug report

Summary

When uploading a file using the Media Browser (accessed from the Media menu on the upper left of the Manager) it strips away the dot that prepends the file type.

Step to reproduce

Launch Media > Media Browser, navigate to and select folder to upload image to, click on the circular black Up Arrow icon or right click on the folder to display and activate "Upload Files" in the contextual menu.

Observed behavior

It strips out the . Thus "ArdenMeg.webp" becomes "ardenmegwebp"
Medi-Browser-error

Expected behavior

Maybe it's not a feature but a bug. When I insert the image into a resource, it displays fine in Firefox, Brave and Safari as "ardenmegwebp"

Environment

MODXcloud 3.0.0 upgrade from 2.8.4

@PeterBcreates PeterBcreates added the bug The issue in the code or project, which should be addressed. label Apr 14, 2022
@opengeek
Copy link
Member

I cannot reproduce this.

@JoshuaLuckers
Copy link
Contributor

I'm also unable to reproduce this issue.

What Media Source are you using? The default "Filesystem" or another?

@PeterBcreates
Copy link
Author

Hopefully these Screenshots will show how this occurred:

I clicked on the Media menu:
media-1

Selected the Filesystem folder to upload the image to, clicked on the upload arrow
media-2

Selected an image:
media-3

It was uploaded without the period/dot:
media-4

@halftrainedharry
Copy link
Contributor

Are there any plugins from other extras that run on the events "OnFileManagerBeforeUpload" or "OnFileManagerUpload" and possibly interfere?

(Under cogwheel-icon -> "System Settings" there is a tab "System Events" to check that. Use the event name in the search field, then check the column "Plugins".)

@PeterBcreates
Copy link
Author

There was one: migxResizeOnUpload. That was a holdover Plugin. I haven't used it in years. I recall removing it a long time ago and used Purged Old Packages thinking that it would do just that.

Since it wasn't in my Package Browser I deleted it by right-clicking on each of the 3 Plugins (MIGX), cleared the cache, logged out, logged back in and uploaded a few images. It still strips out the period mark.

I downloaded MIGX via Package Browser and its install failed, but it did put the Plugins in the correct place. I double checked System Events and migxResizeOnUpload appeared. It was listed in the Package Browser as uninstalled, so I removed it, deleted the Plugins by right-clicking on them, cleared the cache, logged out, logged in, Used the Media Browser and it still strips the period mark.

For good measure I used SFTP to see if the Package was there; it wasn't.

@halftrainedharry
Copy link
Contributor

I deleted it by right-clicking on each of the 3 Plugins

You could just deactivate the plugin. There is a checkbox "Inactive plugin".


Does it make any difference if you change the value of the system setting upload_translit?


For testing purposes you could create a new plugin with this code

<?php
$file = $modx->event->params['files']['file'];
$modx->log(modX::LOG_LEVEL_ERROR,'Event='.$modx->event->name."|Filename=".$file['name']);

and select the events "OnFileManagerBeforeUpload" and "OnFileManagerUpload" in the tab "System Events". This logs the file name (to the error log) on those two events. Maybe this gives some indication where the name gets changed.

@PeterBcreates
Copy link
Author

PeterBcreates commented Apr 18, 2022

Changing the value of upload_translit to No did the trick. The period mark is intact along with the file name as is.
Was there a Plugin or Extra that would have set that to Yes?

@halftrainedharry
Copy link
Contributor

Was there a Plugin or Extra that would have set that to Yes?

"Yes" is the default value.


Remains to find out why the translit doesn't work correctly.
Can you share the values of the system settings 'use_multibyte', 'modx_charset' and all settings starting with 'friendly_alias_'?

@PeterBcreates
Copy link
Author

Sorry for the delay in replying. A lot going on.

Screen Shot 2022-04-20 at 7 29 34 AM

Screen Shot 2022-04-20 at 7 29 57 AM

Screen Shot 2022-04-20 at 7 30 31 AM

@halftrainedharry
Copy link
Contributor

@PeterBcreates Thanks for the information.
Unfortunately these settings have their default values. So I still can't reproduce the issue and it remains a mystery to me, why the dot is stripped from the name.

@JoshuaLuckers
Copy link
Contributor

@PeterBcreates does it happen with all files or only for files with the webp extension?

Do you see anything in the error log? Also check the Manager Log (Manage --> Reports --> Manager log) after uploading the file and share the entries related to file uploads and rename.

We're determined to figure out what's going on here!

@PeterBcreates
Copy link
Author

@halftrainedharry: Reviewing this issue:
upload_translit: default setting is Yes. IfIUC, this will upload files as is, nothing gets stripped out, nothing is lower case.
When I set upload_translit to Yes, the files are not transliterated, and the dot is stripped.

Here's another potential clue I came upon just now. I've uploaded various types of files and the dot is stripped away.
When I try to delete them it says that `` file extension is not allowed. Same error occurs when I try to rename a file without its extension. Maybe that's expected behavior. Here's the screenshot.
deleting-file

@JoshuaLuckers the error report is interesting? 561 lines from just today, of which 501 contain this error:
(ERROR @ /www/core/src/Revolution/modParser.php : 509) Could not find snippet with name Wayfinder.
Wayfinder is installed. Note: 23,193 of 27,036 since 3.30.2022 contain this Wayfinder error.

I set upload_translit to Yes to see what error it generated after I uploaded a file.

  1. Downloaded the error report.
  2. set the value to 'Yes'.
  3. Used Media Browser tool from the left Media menu to upload a file.
  4. Downloaded the error report to compare it to the previous report.
    These two lines were added:
    [2022-04-21 14:22:40] (ERROR @ /www/core/vendor/xpdo/xpdo/src/xPDO/Cache/xPDOCacheManager.php : 498) PHP warning: unlink(/www/core/cache/db/objects/MODX/Revolution/modDeprecatedMethod/c235dfcd703546ae0146eda8dcbd8ef5.cache.php): No such file or directory
    [2022-04-21 14:22:40] (ERROR @ /www/core/vendor/xpdo/xpdo/src/xPDO/Cache/xPDOCacheManager.php : 506) PHP warning: closedir(): supplied resource is not a valid Directory resource

I think someone with more experience needs to login to my cloud instance and have a look at the report as I have no reference for it to make sense. For example, Package Browser says Wayfinder is installed but Wayfinder doesn't show in the Snippets tree.

At some point I'm going to have rebuild these contexts from scratch into a new install. It's like 12 years old, Contexts and Extras have come and gone and there doesn't seem to be a way to manage dependencies, let alone extract just a single Context and transport it to a new install...or is there?

@halftrainedharry
Copy link
Contributor

To make the testing easier: Can you create a snippet "TestTranslit" with this code

<?php
return $modx->filterPathSegment($filename);

then call it with

[[!TestTranslit? &filename=`SomeName.webp`]]

and check if the output from the snippet is also missing the dot?


For example, Package Browser says Wayfinder is installed but Wayfinder doesn't show in the Snippets tree.

Maybe you just accidentally deleted the snippet. Try reinstalling the Wayfinder extra.

@halftrainedharry
Copy link
Contributor

Can you change the snippet code for "TestTranslit" to this and then run it again?
(It's basically the code of the filterPathSegment() function, but stores the different values of the filename in an array.)

<?php
$segment = $filename;
$output = [];

/* setup the various options */
$iconv = function_exists('iconv');
$mbext = function_exists('mb_strlen') && (boolean)$modx->getOption('use_multibyte', $options, false);
$charset = strtoupper((string)$modx->getOption('modx_charset', $options, 'UTF-8'));
$delimiter = $modx->getOption('friendly_alias_word_delimiter', $options, '-');
$delimiters = $modx->getOption('friendly_alias_word_delimiters', $options, '-_');
$maxlength = (integer)$modx->getOption('friendly_alias_max_length', $options, 0);
$stripElementTags = (boolean)$modx->getOption('friendly_alias_strip_element_tags', $options, true);
$trimchars = $modx->getOption('friendly_alias_trim_chars', $options, '/.' . $delimiters);
$restrictchars = $modx->getOption('friendly_alias_restrict_chars', $options, 'pattern');
$restrictcharspattern = $modx->getOption('friendly_alias_restrict_chars_pattern', $options,
    '/[\0\x0B\t\n\r\f\a&=+%#<>"~`@\?\[\]\{\}\|\^\'\\\\]/');
$lowercase = (boolean)$modx->getOption('friendly_alias_lowercase_only', $options, true);
$translit = $modx->getOption('friendly_alias_translit', $options, $iconv ? 'iconv' : 'none');
$translitClass = $modx->getOption('friendly_alias_translit_class', $options, 'translit.modTransliterate');

/* strip html and optionally MODX element tags (stripped by default) */
if ($modx instanceof modX) {
    $segment = $modx->stripTags($segment, '', $stripElementTags ? [] : null);
}
$output["A"] = $segment;

/* replace &nbsp; with the specified word delimiter */
$segment = str_replace('&nbsp;', $delimiter, $segment);
$output["B"] = $segment;

/* decode named entities to the appropriate character for the character set */
$segment = html_entity_decode($segment, ENT_QUOTES, $charset);
$output["C"] = $segment;

/* prepare '&' replacement */
if ($modx instanceof modX && $modx->getService('lexicon', modLexicon::class) && $modx->lexicon('and')) {
    $ampersand = ' ' . $modx->lexicon('and') . ' ';
} else {
    $ampersand = ' and ';
}

/* apply transliteration as configured */
switch ($translit) {
    case '':
    case 'none':
        /* no transliteration */
        break;
    case 'iconv':
        /* if iconv is available, use the built-in transliteration it provides */
        $segment = iconv($mbext ? mb_detect_encoding($segment) : $charset, $charset . '//TRANSLIT//IGNORE',
            $segment);
        $ampersand = iconv($mbext ? mb_detect_encoding($segment) : $charset, $charset . '//TRANSLIT//IGNORE',
            $ampersand);
        break;
    case 'iconv_ascii':
        /* if iconv is available, use the built-in transliteration to ASCII it provides */
        $segment = iconv(($mbext) ? mb_detect_encoding($segment) : $charset, 'ASCII//TRANSLIT//IGNORE',
            $segment);
        break;
    default:
        /* otherwise look for a transliteration service class that will accept named transliteration tables */
        if ($modx instanceof modX) {
            $translitClassPath = $modx->getOption('friendly_alias_translit_class_path', $options,
                $modx->getOption('core_path', $options, MODX_CORE_PATH) . 'components/');
            if ($modx->getService('translit', $translitClass, $translitClassPath, $options)) {
                $segment = $modx->translit->translate($segment, $translit);
                $ampersand = $modx->translit->translate($ampersand, $translit);
            }
        }
        break;
}
$output["D"] = $segment;

/* replace any remaining '&' with a translit ampersand */
$segment = str_replace('&', $ampersand, $segment);
$output["E"] = $segment;

/* restrict characters as configured */
switch ($restrictchars) {
    case 'alphanumeric':
        /* restrict segment to alphanumeric characters only */
        $segment = preg_replace('/[^\.%A-Za-z0-9 _-]/', '', $segment);
        break;
    case 'alpha':
        /* restrict segment to alpha characters only */
        $segment = preg_replace('/[^\.%A-Za-z _-]/', '', $segment);
        break;
    case 'legal':
        /* restrict segment to legal URL characters only */
        $segment = preg_replace('/[\0\x0B\t\n\r\f\a&=+%#<>"~`@\?\[\]\{\}\|\^\'\\\\]/', '', $segment);
        break;
    case 'pattern':
    default:
        /* restrict segment using regular expression pattern configured (same as legal by default) */
        if (!empty($restrictcharspattern)) {
            $segment = preg_replace($restrictcharspattern, '', $segment);
        }
}
$output["F"] = $segment;

/* replace one or more space characters with word delimiter */
$segment = preg_replace('/\s+/u', $delimiter, $segment);
$output["G"] = $segment;

/* replace one or more instances of word delimiters with word delimiter */
$delimiterTokens = [];
for ($d = 0; $d < strlen($delimiters); $d++) {
    $delimiterTokens[] = preg_quote($delimiters[$d], '/');
}
if (!empty($delimiterTokens)) {
    $delimiterPattern = '/[' . implode('|', $delimiterTokens) . ']+/';
    $segment = preg_replace($delimiterPattern, $delimiter, $segment);
}
$output["H"] = $segment;

/* unless lowercase_only preference is explicitly off, change case to lowercase */
if ($lowercase) {
    if ($mbext) {
        /* if the mb extension is available use it to protect multi-byte chars */
        $segment = mb_convert_case($segment, MB_CASE_LOWER, $charset);
    } else {
        /* otherwise, just use strtolower */
        $segment = strtolower($segment);
    }
}
$output["I"] = $segment;
/* trim specified chars from both ends of the segment */
$segment = trim($segment, $trimchars);
$output["J"] = $segment;

/* get the strlen of the segment (use mb extension if available) */
$length = $mbext ? mb_strlen($segment, $charset) : strlen($segment);

/* if maxlength is specified and exceeded, return substr with additional trim applied */
if ($maxlength > 0 && $length > $maxlength) {
    $segment = substr($segment, 0, $maxlength);
    $segment = trim($segment, $trimchars);
}
$output["K"] = $segment;
return json_encode($output);

At which point disappears the dot in the name?


btw: You posted your last answer in the wrong thread/issue.

@PeterBcreates
Copy link
Author

Sorry about posting in wrong issue.
The dot disappears at F:
TransLit-results

@halftrainedharry
Copy link
Contributor

Ok, so the problem seems to be the system setting friendly_alias_restrict_chars_pattern.

This is the value I have on a fresh MODX3 installation: /[\0\x0B\t\n\r\f\a&=+%#<>"~:`@\?\[\]\{\}\|\^'\\]/

When I look at your printscreen further above, I can see that in your setting there is a . right after <>, that shouldn't be there. I didn't pay close enough attention when you posted your settings, as the lack of a "Last Modified" date implied that the value was unchanged.

If you remove this dot from the setting, your upload should work as expected.

@rthrash
Copy link
Member

rthrash commented Apr 22, 2022

Great eye @halftrainedharry 🕵️‍♂️ … you're going to be scary-good when the other half of your training is complete!

@PeterBcreates
Copy link
Author

Yep, that works. I don't know how that got there. I think your training is complete. What's your Venmo so I can buy you coffee...not that you need it but just to express my gratitude so I can stop feeling halfwaystupid.

@halftrainedharry
Copy link
Contributor

@PeterBcreates There's no Venmo where I live, but thanks for the offer. 😀

@rthrash
Copy link
Member

rthrash commented Apr 22, 2022

Thanks for sorting this out @halftrainedharry … closing.

@rthrash rthrash closed this as completed Apr 22, 2022
@JoshuaLuckers
Copy link
Contributor

@PeterBcreates and @halftrainedharry thank you both for taking time to solve this issue! :D

@modxcommunity
Copy link
Collaborator

This issue has been mentioned on MODX Community. There might be relevant details there:

https://community.modx.com/t/bug-system-setting-upload-translit-removes-before-file-extension/7355/2

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug The issue in the code or project, which should be addressed.
Projects
None yet
Development

No branches or pull requests

6 participants