User tests: Successful: Unsuccessful:
JFile::getExt works incorrect if file does not have extension.
Status | New | ⇒ | Pending |
Category | ⇒ | Libraries |
Labels |
Added:
?
|
You can not reproduce this problem in the CMS, only in an extension. But you can write php-code that uses it: $ext = JFile::getExt ('test');
Result: $ext = "est'
Right result: $ext = ''
Maybe a false
is a better return value, if there is no extension. is returned if there is a dot as last letter in the string. So
false
would indicate that there is no dot.
I've tested JFile::getExt('test.')
. It returns false
, but it should be .
I confirm the issue and the fix,
@marenkov , Joomla code must follow specific code styling
if ($dot === false)
{
return '';
}
About
Maybe a false is a better return value, if there is no extension. is returned if there is a dot as last letter in the string. So false would indicate that there is no dot.
Filename with trailing dot and without extension is usually not valid,
although some programs might create it, e.g. if you create such a name in windows it will be difficult to use or delete it ...
What about
/**
* Gets the extension of a file name
*
* @param string $file The file name
*
* @return string The file extension
*
* @since 11.1
*/
public static function getExt($file)
{
return strtolower(pathinfo($file, PATHINFO_EXTENSION));
}
Based on: https://github.com/joomla-framework/filesystem/blob/master/src/Stream.php#L243
Nearly perfect, but there is the code style issue...
The following line condenses your two lines to one and solves the code style issue:
return $dot === false ? false : (string) substr($file, $dot + 1);
Recently i was trying to remember why i was not using
JFile::getExt
and instead i am using:
pathinfo($file, PATHINFO_EXTENSION)
@zero-24
just, we cannot use strlower(), because
about
return $dot === false ? false : (string) substr($file, $dot + 1);
it seems good, but better not rediscover the wheel, for no good reason,
instead just use:
return pathinfo($file, PATHINFO_EXTENSION);
return pathinfo($file, PATHINFO_EXTENSION);
looks good, but it returns string(0) for 'test.' and 'test' (instead false).
If it is not problem, it is the best solution.
I have tested this item
I have tested this item
Status | Pending | ⇒ | Ready to Commit |
Labels |
Added:
?
|
Sorry guys, but I don't think we should do it this way. I don't like the different return types
JFile::getExt('whatever')
should return an empty string
JFile::hasExt('whatever')
should return true or false (we don't have such a function atm and I am not saying we should make one :-))
Status | Ready to Commit | ⇒ | Needs Review |
Labels |
Removed:
?
|
set to needs review
don't like the different return types
should return an empty string
Ok then, lets not make 2 changes in this PR, (1 fix + 1 behaviour change)
Let it return string as before, and ... only fix the original reported bug:
// Check for no extension
if ($dot === false)
{
return '';
}
In anyway current realization is wrong. We have two version of fix:
My version - we need select what is returned for files without extension and with dot at the end.
pathinfo - always returns string(0) if extension is empty. Not bad.
Somebody should select and fix it.
I made a decision as maintainer, return an empty string instead of false
OK. I've changed false to empty string.
Status | Needs Review | ⇒ | Ready to Commit |
Thanks back to RTC
Labels |
Added:
?
|
Status | Ready to Commit | ⇒ | Fixed in Code Base |
Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2016-09-27 15:06:06 |
Closed_By | ⇒ | rdeutz |
Labels |
Removed:
?
|
Please add description how to reproduce the issue, and how to test the fix.