Any code that calls getOptions() in libraries/joomla/form/fields/filelist.php, where $this->directory is NOT a relative path, in my example "/images/virtuemart/payment".
No is_dir() warning
Warning: is_dir(): open_basedir restriction in effect. File(/images/virtuemart/payment) is not within the allowed path(s): (/var/www/vhosts/XXXXX.com/:/tmp/) in /var/www/vhosts/XXXX.com/httpdocs/libraries/joomla/form/fields/filelist.php on line 193
Joomla 3.10.6
PHP 7.4.27
Workaround:
I replaced
$path = $this->directory;
if (!is_dir($path))
{
$path = JPATH_ROOT . '/' . $path;
}
with
$path = JPATH_ROOT . '/' . $this->directory;
Labels |
Added:
No Code Attached Yet
|
Labels |
Added:
J3 Issue
|
@dgrammatiko Thanks for the correction – I meant to say, "is NOT a relative path", now fixed. The leading "/" is what causes is_dir to throw the warning. I think the function is meant to handle both cases, leading "/" and no leading "/".
RE:
Also you SHOULD NEVER, EVER put executable files (eg .php) inside the images or the media folders from a security point of view...
I don't know why you are mentioning this, because it has no bearing on this bug report, correct?
it has no bearing on this bug report, correct?
Because it's a potential security issue. The folders images
and media
are supposed to only contain static assets, nothing .php
...
Dunno why the heck you are saying that. Are you talking about this .php file?
libraries/joomla/form/fields/filelist.php
This is the code that has the bug on line 193.
Again, what you are saying has no bearing on this bug report.
There is no bug. Your code is wrong, remove the first slash
No, you are wrong. The function is designed to handle leading "/" or no leading "/". My workaround is the correct solution**. You need to look more closely at this, or get out of the way so another developer can do that.
And why don't you admit that you are also wrong for mentioning this, because it has nothing to do with the bug report?
Because it's a potential security issue. The folders images and media are supposed to only contain static assets, nothing .php
Your attitude is not helpful here.
** UPDATE: Not necessarily completely correct because $this->directory may possibly be a full path from the server's root.
a relative path, in my example "/images/virtuemart/payment".
Again you shouldn't roll PHP files in the images or media folder. Which part of what I wrote you didn't understand?
Dude, I'm gonna have to report you!
Do these look like .php files to you?
bank-transfer-logo.png
Paypal-Logo.png
am i being thick - why not just change your code to use the correct relative path?
** CORRECTED **
Clearly, the function should handle, and I think is intended to handle, the case of leading "/" and not leading "/".
The function is actually using is_dir() here to determine if JPATH_ROOT needs to be added to the front of the path.
If is_dir() returns FALSE, this is the case where $this->directory has a leading "/" even though it's a directory in the Joomla tree, as in my example, "/images/virtuemart/payment"; and it would happen as well with a fully-qualified path, from the server's root, where that path does not resolve to a valid directory.
$path = $this->directory;
if (!is_dir($path))
{
$path = JPATH_ROOT . '/' . $path;
}$path = JPath::clean($path);
JPath::clean() converts any "//" to "/".
It's a separate issue to check if the resultant $path is to a valid directory, and that's when is_dir() should be used. If is_dir() then fails, I think the function should return a failure because it was passed an invalid directory path.
This is not my code by the way, it's VirtueMart. Wasn't broken before, I believe until a newer PHP version.
I am noting here that @dgrammatiko has not yet offered an apology.
Apology for what? I already said in my first comment that this is not a Joomla's bug. Also I added a comment about not using php files in directories for assets because you never actually provided enough code to see what this code was serving. BTW for images Joomla has specific API's that also offer overrides for the images, so that code is not only wrong because it's using absolute path instead of relative it also goes against the platform as it neglects the overriding aspect of static assets.
Ah geez, please @brianteeman.
Here's how a correct use of this field looks like:
joomla-cms/administrator/components/com_languages/forms/language.xml
Lines 47 to 59 in 1df9d40
So, your assumption that this is a Joomla's bug was wrong (what I pointed out in my first comment). You should create an issue to the component that has the bug.
This should be closed as it's not a bug, Joomla has this code with this specific functionality for over 10 years and I don't see any benefits changing the method to accept absolute paths as relative.
My 2c, and I'm unsubscribing
My position is that the programming of the function should be intelligent to anticipate either leading "/" or not leading "/", and I believe that was the intention of the authors of that function – I mean, it's quite clear...
$path = JPATH_ROOT . '/' . $path;
With a new PHP version, is_dir() started throwing these warnings, as has been addressed in #35895 #36788. This means many extensions that previously didn't see this warning are now seeing this warning.
Joomla functions should not rely on is_dir() throwing a warning to notify users that they've entered a path beginning with "/" when they should have used a full path, or a path relative to the Joomla root. Joomla functions should return errors to the caller when an invalid directory has been passed.
The purpose of is_dir() is to check if a directory path is valid... i.e. that the path isn't to a file, and that the directory exists and is accessible in the Joomla tree.
There are a great many instances of is_dir() being used in the Joomla core.
@dgrammatiko I think you misunderstood the error message:
Warning: is_dir(): open_basedir restriction in effect. File(/images/virtuemart/payment) is not within the allowed path(s): (/var/www/vhosts/XXXXX.com/:/tmp/) in /var/www/vhosts/XXXX.com/httpdocs/libraries/joomla/form/fields/filelist.php on line 193
The File(/images/virtuemart/payment)
is the path in question, and the filelist.php
is the source which want to use that path, i.e. it is the file where the error happens, not the one for which the error happens. The author did not try to use a PHP file in his images folder.
@durian808 We all can make mistakes sometimes, and if someone makes a mistake, we do not insist on something like an apology. You should not do that either.
@richard67 Thanks for providing backup, because I couldn't get through to Mr. Grammatiko.
We all can make mistakes sometimes, and if someone makes a mistake, we do not insist on something like an apology. You should not do that either.
Oh I fully understand that; however, Mr. Grammatiko was insistent and oblivious. Sometimes people who behave like that need a wake-up call. So, I'll throw this right back at you... YOU shouldn't be telling ME, "You should not do that either." Because you're not my daddy.
And, I was not "insisting" on an apology, I was simply stating that Mr. Grammatiko didn't offer one, where one would have been typically expected, and so I was noting that for the record. Something like, "Oh I see, sorry about that." Nothing of the sort from Mr. Grammatiko. No simple, courteous admission of a mistake.
The golden rule applies here, and had it been me with someone trying to tell me to "look again," I would sincerely look again, and then admit that I missed the point. Mr. Grammatiko came in locked and loaded to shoot down anything from the pesky outsider making the bug report. This doesn't bode well for Joomla. I've actually run into this behavior before when posting to this system. Some of the developers are high-and-mighty, seem overworked or burnt out, and... that is reflected in their responses here.
I have a background of over 20 years as a senior level software engineer in data communications and networking system software development and embedded software, and over 15 years of experience as a web developer, PHP programmer, and web hosting provider.
I encourage someone to please look at what I've posted here and to give it some good consideration. Thank you!
@durian808 When checking the documentation to the filelist field here https://docs.joomla.org/Filelist_form_field_type , the example shows directory="administrator"
and not directory="/administrator"
. So it should be clear that a path below the Joomla root should not start with a slash. A path starting with a slash is an absolute path from the webserver's point of view, and so maybe relative to the webserver root but not necessarily be relative to the Joomla root.
In my opinion you should be thankful that you got advice how to fix your code to use the right kind of path instead of making up arguments just for the sake of having been right at the end.
I think you misunderstood the error message:
@richard67 I think I didn't but anyways the comment about not having php files in images or media was a side note, as there wasn't any clues what the filelist had been used for. Also quite weird that this guy triggered by that...
Anyways, about the proposed solution: it's totally wrong and here's an actual example of my own code that using the filelist field to access a folder not in the Joomla root:
/var/www/html/site
is the Joomla root/var/www/html/protected
is a folder that holds zips for a distributed softwaredirectory="/var/www/html/protected/"
is the XML declaration for the directoryThis works with the current code. The idea that Joomla should not allowed to access anything other than what's inside the Joomla root is an artificial limit that only applies to installations on a cheap shared server but Joomla could be used in it's own VPN server, so in that case the limit will be rather annoying.
Mr. Grammatiko still not backing down. Unbelievable. @brianteeman are you seeing this? Does anyone care?
It's not my code, it's Virtuemart, and there are many Joomla extensions that are affected by the change in the behavior of is_dir() and is_file() with the newer version of PHP. You people don't seem to care about how this is all functioning in the real world. Did you even look at the fact that the code is adding JPATH_ROOT to the beginning of the path? Why do you think the original authors did that?
it's Virtuemart
Then OPEN A ISSUE THERE...
A "side note". Unbelievable. It was completely irrelevant to the bug report and shouldn't have even been mentioned.
THERE IS NO BUG
No, no, no. Look... If you guys can't understand this, then get a developer in here who does.
Grammatiko said he was unsubscribing... do it, brother.
First of all I'm not your brother and if you tagging me you inviting me back. Have you ever used GitHub?
Wrong again dude. You never unsubscribed. What a joke.
Status | New | ⇒ | Closed |
Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2022-03-13 12:03:54 |
Closed_By | ⇒ | chmst |
Please be polite, @durian808.
Your issue is not a bug, so I close this.
Does Grammatiko get a "be polite" too? Or is he above that because he's got some kind of seniority here?
Status | Closed | ⇒ | New |
Closed_Date | 2022-03-13 12:03:54 | ⇒ | |
Closed_By | chmst | ⇒ |
I reopen this ticket, beside the rudeness of the ticket opener the concerns are failed the proposed solution seems to be wrong.
assuming that the path is relative because it's is_dir returns false seems wrong to me. Always prefixing JPATH_ROOT seems also wrong because it's possible that the developer like to haven an absolute path or a directory outside the joomla root directory.
My suggestion would be to check if $path
starts with a /
and depending on this add the JPATH_ROOT if it's not starting with a /
. In this case I would expect the developer want's an absolute path and Joomla shouldn't add magic here.
in the end the Joomla! behavior is some kind of magic which hasn't been noticed since 12 years but should be changed to do the right thing anyway.
Which would not solve the problem of the issue starter but clarify the way it should work.
in the end the Joomla! behavior is some kind of magic which hasn't been noticed since 12 years but should be changed to do the right thing anyway.
Which would not solve the problem of the issue starter but clarify the way it should work.
While that may be valid (I havent checked but it probably is) it doesnt change the fact that the only way to fix the original problem is to change the code in the plugin
What @HLeithner proposed is changing
joomla-cms/libraries/src/Form/Field/FilelistField.php
Lines 192 to 199 in cb07dbd
to
$path = Path::clean(strpos($this->directory, '/') === 0 ? $this->directory : JPATH_ROOT . '/' . $this->directory);
if (!$path) {
throw new \Exception('nope');
}
But as @brianteeman and @HLeithner that is not solving the problem reported here, which is wrong use of the API
of course because I see no way that we can fix the problem in the plugin without changing the behavior for all other usages
in the end the Joomla! behavior is some kind of magic
Nope its the way that is_dir is intended to work
is_dir works as expected but why uses joomla such a strange way to detect if it is a relative path? that makes no sense to me.
because if I have a images
directory in my root folder that function/magic to add the JPATH_ROOT would fail
@HLeithner there you go: #37261
btw I'm not sure if this would work on fat/ntfs machines, if it doesn't just close it
Status | New | ⇒ | Closed |
Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2022-03-13 17:33:06 |
Closed_By | ⇒ | HLeithner |
Closing since we have a PR, thanks dimitris
Your path IS NOT relative, it is absolute. If you want to define a relative directory then it should be
images/virtuemart/payment
(missing the first/
)Also you SHOULD NEVER, EVER put executable files (eg .php) inside the images or the media folders from a security point of view...