? Success

User tests: Successful: Unsuccessful:

avatar mgilkes
mgilkes
8 Nov 2016

Pull Request for Issue # .

Summary of Changes

On line 219, the root folder path is trimmed on the end of both back and forward slashes, and the resulting folder is trimmed of both back and forward slashes.

Testing Instructions

The changes address the issue of folderlist on Windows-based web servers. You can test this on IIS or WAMP, just access any module (or plugin) that uses folderlist, and you will see "C:\wamp\www/images" or "C:\inetpub\wwwroot/images", when it should only show as "images".

Since there are no built-in extensions that use folder list, you can add the following field to any module's xml file to test it:

<field name="test_field" type="folderlist" default="images" label="TEST_LABEL" directory="" description="TEST_DESC" hide_none="true" hide_default="true" exclude="^[Aa]dministrator$|^[Cc]ache$|^[Cc]omponents$|^[Cc]li$|^[Ii]ncludes$|^[Ll]anguage$|^[Ll]ibraries$|^[Ll]ogs$|^[Mm]odules$|^[Pp]lugins$|^[Tt]emplates$|^[Xx]mlrpc$" />

Documentation Changes Required

On non-windows-based web servers, the current code works fine, but on windows-based web servers, like IIS or WAMP, the folderlist options will look like "C:\wamp\www/images" or "C:\inetpub\wwwroot/images", when it should only show as "images". This messes up any extension that uses folderlist on windows-based web servers.

I propose to change line 219 from:
$folder = trim(str_replace($path, '', $folder), '/');
to:
$folder = trim(str_replace(rtrim($path, '/\\'), '', $folder), '/\\');

This essentially accommodates Windows systems to make the folderlist work as it should.

avatar mgilkes mgilkes - open - 8 Nov 2016
avatar mgilkes mgilkes - change - 8 Nov 2016
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 8 Nov 2016
Category Libraries
avatar brianteeman
brianteeman - comment - 8 Nov 2016

Is this the same as #11524 and #11288 ??

avatar mgilkes
mgilkes - comment - 9 Nov 2016

This issue still exists in Joomla 3.6.4. My solution is different from those proposed. One of the problems with the current code is that the $path variable string actually has a trailing slash, which is why the folder root path is not removed by the str_replace on line 219. It should be trimmed.

avatar ggppdk ggppdk - test_item - 11 Nov 2016 - Tested successfully
avatar ggppdk ggppdk - test_item - 11 Nov 2016 - Tested unsuccessfully
avatar ggppdk
ggppdk - comment - 11 Nov 2016

I have tested this item 🔴 unsuccessfully on 972fa74

This PR does not work in all cases

If the original folder has "/" then JFolder will "clean" it thus replace the "/" with "\" (windows seperator)
in that case the str_replace to remove the given original folder and make subpath relative, will fail


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/12841.

avatar mgilkes mgilkes - change - 11 Nov 2016
The description was changed
avatar mgilkes mgilkes - edited - 11 Nov 2016
avatar mgilkes
mgilkes - comment - 11 Nov 2016

@ggppdk I don't quite understand your comment. When you say "original folder" are you referring to the value of $path or $folder (just before line 219)?

Because $path is the root path of the folder list, and $folder is the full path of a subfolder in the folder list, the purpose of line 219 is to remove the root path from the full path, and leave only the subfolder, without any slashes before or after it. Could you better clarify the use case in which:

$folder = trim(str_replace(rtrim($path, '/\\'), '', $folder), '/\\');

would fail to provide the subfolder name? Thanks.


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/12841.

avatar ggppdk
ggppdk - comment - 11 Nov 2016

JFolder::folders($path, ...

Returns:
Folders LIKE: CLEANED($path) /somefolder1/someofolder2

  • thus CLEANED($path) will contain current server directory separator, thus it will contain \

Now this
$folder = trim(str_replace(rtrim($path, '/\'), '', $folder), '/\');
will fail if $path has any INNER / and you are in windows

e.g. thus you PR will fail with

<field ... type="folderlist" ...  directory="images/sampledata"

but you PR will fail even fail with:

<field ... type="folderlist" ...  directory="images" 

because folderlist does:

if (!is_dir($path))
{
    $path = JPATH_ROOT . '/' . $path;
}

In any case please test this PR: #11288
(it has conflicts and needs to be rebased) , it would have been best if you had searched and post at that PR, but i guess you missed it

avatar mgilkes
mgilkes - comment - 12 Nov 2016

Thanks for the clarification. You are right. I think the best thing to do is to clean the $path variable before using it to find the subfolders with JFolder. See my update: f73febd


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/12841.

avatar wilsonge
wilsonge - comment - 23 Feb 2017

Closing this in favour of #14139 as we had 4 different PR's for the same issue :)

avatar wilsonge wilsonge - change - 23 Feb 2017
Status Pending Closed
Closed_Date 0000-00-00 00:00:00 2017-02-23 01:32:29
Closed_By wilsonge
avatar wilsonge wilsonge - close - 23 Feb 2017

Add a Comment

Login with GitHub to post a comment