[#3592] - Don't assume M as suffix for size configuration values in admin/com_media
- Closed
- 26 Jul 2014
- Medium
- Build: staging
- # 3592
- Diff
- dmbwien:staging
- Success The Travis CI build passed Details
User tests: Successful: Unsuccessful:
Instead take the possible values defined in the php configuration into
account (k,m,g and upper case as well). The function is the optimized
version found on php.net but using brackets and strlen() to access the
last character instead of substr().
This change fixes an upload issue in the MediaManager when any
configuration setting uses something different than the assumed 'M/m'
suffix. A small example should make this clearer:
Imagine 'memory_limit' being set to '1G'.(int) (ini_get('memory_limit')) * 1024 * 1024 would resolve to
1048576 Bytes (~=1M), a value (almost certainly) being lower than
most of the media files an average user would like to upload. ;-)
Would it make more sense to move the toBytes($val) method to the JHelperMedia class?
Please also make sure you follow our coding standards (http://joomla.github.io/coding-standards/?coding-standards/chapters/php.md).
Would it make more sense to move the toBytes($val) method to the JHelperMedia class?
Definitively. I'll update the code accordingly taking
Please also make sure you follow our coding standards (http://joomla.github.io/coding-standards/?coding-standards/chapters/php.md).
your coding standards into account. ;-)
(Btw, those should be placed more prominently, all I found were some dead links and/or depreceated pages.)
Thx for the quick reply.
Btw, those should be placed more prominently, all I found were some dead links and/or depreceated pages
Agreed. And I actually want to improve the contribute.md for quite some time. Just have to do it...
I wouldn't add the proxy function to administrator/components/com_media/helpers/media.php. The whole class is deprecated, so adding stuff is probably no good idea. 
As for the function name, maybe convertToBytes or something. However toBytes is fine for me as well.
I wouldn't add the proxy function to administrator/components/com_media/helpers/media.php. The whole class is deprecated, so adding stuff is probably no good idea.
True, but JMediaHelper isn't imported currently (as far as I can see it's not really in use currently) and as all other MediaHelper functions are still in place there I thought it'd can be moved later on transparently with the other calls.
As for the function name, maybe convertToBytes or something. However toBytes is fine for me as well.
I'll leave it as is then. 
True, but JMediaHelper isn't imported currently
It should autoload
I was in the midst of answering you when it dawned me that I should instantiate it and not call it statically. 
Fix incoming…
| Status | New | ⇒ | Closed |
| Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2014-07-26 10:57:29 |
Issue-Tracker ID: #33734