-
-
Notifications
You must be signed in to change notification settings - Fork 452
Description
Summary
magento-lts/app/code/core/Mage/Adminhtml/Model/System/Config/Backend/File.php
Lines 140 to 149 in 3901dad
| protected function _getUploadRoot($token) | |
| { | |
| $value = Mage::getStoreConfig($token) ?? ''; | |
| if (strlen($value) && preg_match(self::SYSTEM_FILESYSTEM_REGEX, $value, $matches) !== false) { | |
| $dir = str_replace('root_dir', 'base_dir', $matches[1]); | |
| $path = str_replace('/', DS, $matches[2]); | |
| return Mage::getConfig()->getOptions()->getData($dir) . $path; | |
| } | |
| return Mage::getBaseDir('media'); |
Explanation: The Token System and Media Directory
The token system is configuration-based and provides flexibility, while the hardcoded Mage::getBaseDir('media') is fixed. This would create a huge problem if the token system were to be used because it is not consistent with the codebase use of the fixed method. The use of the token system should be removed.
How The Token System Works
The Config Path Token
The token 'system/filesystem/media' is a config path that points to a value stored in the database (or config files). Its default value is:
<media>{{root_dir}}/media</media>Step-by-step of _getUploadRoot()
-
Fetch config value:
Mage::getStoreConfig('system/filesystem/media')returns{{root_dir}}/media -
Parse the token: The regex
/{{([a-z_]+)}}(.*)/matches:$matches[1]=root_dir$matches[2]=/media
-
Convert to Options key:
root_dir→base_dir(line 228) -
Build path:
- Get base directory:
Mage::getConfig()->getOptions()->getData('base_dir') - Append path:
+ '/media' - Result:
/home/kiat/openmage/media
- Get base directory:
-
Fallback: If the config value doesn't exist or doesn't match the pattern, it defaults to
Mage::getBaseDir('media')
Why It's Bad if It's Used
By default, both methods return the same value:
Mage::getBaseDir('media')→/path/to/openmage/media_getUploadRoot('system/filesystem/media')→ parses{{root_dir}}/media→/path/to/openmage/media
The token system allows administrators to customize upload directories through the admin panel or config without changing code. For example, an admin could:
- Change
system/filesystem/mediato{{base_dir}}/custom_media - Point to a different mount:
{{base_dir}}/mnt/shared_storage - Use different directories per store scope
The Problem
Statistics:
- 32 uses of
Mage::getBaseDir('media')across 25 files throughout the core codebase - Only 3 files use
_getUploadRoot()with the token system (all in System Config Backend)
If someone configures a custom media directory:
- ✅ Admin file uploads (logo, watermarks, placeholders) → Would go to the configured location
- ❌ Everything else → Would still use the hardcoded
/mediadirectory:- Product images:
Mage_Catalog_Model_Product_Media_Config - Product attributes with images
- Downloadable products
- CMS WYSIWYG images
- Email templates with images
- PDF generation (invoices, etc.)
- Customer file uploads
- Captcha images
- Design/skin assets
- File storage system
- Product images:
Real Examples of the Issue
return Mage::getBaseDir('media') . DS . 'catalog' . DS . 'product';$config['media_directory'] = Mage::getBaseDir('media');These would always use the hardcoded path, ignoring any system/filesystem/media configuration.
Why This Exists
This appears to be a half-implemented feature from Magento 1.x that was never fully realized:
- The
system/filesystem/config was defined inapp/etc/config.xml(lines 108-124) - The token parsing system was built into File backend models
- But: There's no admin UI to configure these paths
- And: The vast majority of code ignores it
The Reality
The system/filesystem/media token is not a supported configuration option in practice. It's only used internally by a few admin upload fields, and even then, it defaults back to Mage::getBaseDir('media').
Changing this config would break:
- ❌ Product image display/upload
- ❌ Category images
- ❌ CMS media browser
- ❌ Downloadable products
- ❌ PDF generation
- ❌ Customer uploaded files
- ❌ Database media storage sync
- ✅ Admin system config file uploads (only thing that would work)
Recommendation
Don't configure a custom media directory through system/filesystem/media. If you need to change the media location, you should:
- Use symlinks at the filesystem level
- Use a CDN with proper URL configuration
- Use the built-in Database Storage feature for media
- Modify the hardcoded path in
Mage_Core_Model_Config_Options::_construct()if absolutely necessary
The token system is a legacy artifact that shouldn't be relied upon. The actual, functional way to get the media directory is and always should be Mage::getBaseDir('media').
Examples
See above.
Proposed solution
See above.
Anything else?
No response