Unify the browser and upload connectors

[Alfonso Martínez de Lizarrondo]

Now there are two connectors folders for each server language, one for the quick upload and another one for the filemanager.

That means duplicated code and confusion for users because they have enabled only one of them and the other part doesn't work. Not taking into account the problems when a change is done only in one set of folders and the other one isn't updated.

So I propose to use just one set of files and so provide in their configuration files the option to enable or disable now the basic functionality like Upload, Browse files, and then it can be further expanded to enable deletion if the user wants to.

I'll try to work in my branch to make the proper changes to the asp and php connectors and let's see if I'm able to understand other languages.

[Alfonso Martínez de Lizarrondo]

I've checked [286] that does the unification for the asp connector and at the same time it does bring other features:

Theres a new setting in the config file so the user can choose the possible commands enabled for the connector (for example only browse, but not upload or viceversa)

dim ConfigAllowedCommands
ConfigAllowedCommands = "FileUpload|GetFolders|GetFoldersAndFiles|CreateFolder"

There's a new setting to easily set where should each file type go:

Set ConfigFileTypesPath	= CreateObject( "Scripting.Dictionary" )

ConfigFileTypesPath.Add "File", ConfigUserFilesPath & "file/"

ConfigFileTypesPath.Add "Image", ConfigUserFilesPath & "image/"

ConfigFileTypesPath.Add "Flash", ConfigUserFilesPath & "flash/"

ConfigFileTypesPath.Add "Media", ConfigUserFilesPath & "media/"

That way if a user wants to put everything in the same folder then he just needs to change those four lines, or he can make Files go to "/files/mine/", Image to "/userFiles/images/" and Media to "/userFiles/media/", etc...

When all the changes are complete this tracker will fix several Feature request about this issues.

[Alfonso Martínez de Lizarrondo]

I've added some other little improvements from SF Patch 1315722 (avoid getting a cached response) and SF Patch 1662181 (bugfix if the form upload has more data).

This is very similar to the ideas proposed in ​https://sourceforge.net/tracker/index.php?func=detail&aid=1312834&group_id=75348&atid=543655 so I might get the coldfusion changes done more easily and also the java code from ​https://sourceforge.net/tracker/index.php?func=detail&aid=1462144&group_id=75348&atid=543655

[Alfonso Martínez de Lizarrondo]

I've checked in [290] the new php connector, so if anyone else can test these changes that would be great.

I'll try to work on the rest of the connectors, but I won't be able to test them (so if anyone else steps up with the corresponding changes it would be very helpful)

[Thomas Tallyce]

NB See also ​http://dev.fckeditor.net/ticket/306 which I think overlaps with a lot of this stuff. Can you confirm?

[Alfonso Martínez de Lizarrondo]

This will cover the Configurability of subdirectories, I'm also checking existing bug reports to try to fix existing problems, but I don't plan to add new features with this bug.

The reason for the new configuration settings are to make it easier the transition for the people that used only the quick upload feature with one common directory: they can disable the get* commands as well as the createFolder, so end users aren't able to do anything that they didn't want. And instead of porting the useFileType configuration from the upload, I thought that it would be better to bring a single solution to all the complains about were to store the files.

Also this way it's quite easy to bring new features like the ability to delete or rename folders and/or files .

Now most of the changes are finished for the asp and php connectors, although I'll add a new command: GetEnabledCommands so in the fileManager the "create folder" and "upload new file" options can be hidden if they have been disabled.

For the php connector I have to check again the utf-8 thing as I was getting weird results yesterday

[Alfonso Martínez de Lizarrondo]

#454 and #465 have been marked as dups.

[Alfonso Martínez de Lizarrondo]

make that #458

[Alfonso Martínez de Lizarrondo]

I've checked in [300] the unified ColdFusion connector, as it didn't have the option to choose in the upload the ability to use or not the file type I didn't mess too much with it (also because my coldfusion skills are non-existent)

This connector seems to allow passing the configuration data from the request string, and that could be a very dangerous thing and I think it should be removed.

[Alfonso Martínez de Lizarrondo]

The Perl connector has been checked in [301].

Notes: I've changed the line endings in connector.cgi from PC to Unix as it was generating a 500 error for me, and I've added the upload.cgi to be able to output the javascript messages properly for the quickupload.

There isn't still a config.pl file to change the settings without touching the files all the way.

[Frederico Caldeira Knabben]

You have done a great job Alfonso.

In my previous thoughts about it, I've been thinking about creating a new command called "QuickUpload" for this kind of upload, instead of reusing "FileUpload", which would be reserved for the File Browser. Maybe this separation could bring us ways to handle the upload accordingly, by, for example, handling special configurations for the quick upload. Of course, the upload code would be shared between those commands.

One of the reasons for the command separation it that I was wondering that, for backward compatibility, shouldn't we provide a configuration similar to "UseFileType". So when upgrading, our users will not have many surprises.

[Frederico Caldeira Knabben]

Replying to alfonsoml:

Theres a new setting in the config file so the user can choose the possible commands enabled for the connector (for example only browse, but not upload or viceversa)

I would avoid this feature for now because its usage could bring confusion to the end user, as the interface would not reflect the configured settings.

There's a new setting to easily set where should each file type go

This one is cool, as it has been strongly requested previously.

I liked the way you have proposed it initially, by concatenating the "configured base path" with the type path (like ConfigUserFilesPath & "file/" in ASP). But, looking at the sources, you have changed it to "/userfiles/files/", and repeated it for each type.

I would prefer the initial proposal, as it brings a single point of configuration, which would be ok for the majority of users. Then, for those who really need more control for each type, they could go to the specific type and make the changes.

[Frederico Caldeira Knabben]

Replying to alfonsoml:

This connector seems to allow passing the configuration data from the request string, and that could be a very dangerous thing and I think it should be removed.

It it certainly something to me removed.

We just only remember all those things, to list them in the ​Upgrade Instructions page.

[Frederico Caldeira Knabben]

Please consider #273 in this ticket too to avoid conflict.

[Alfonso Martínez de Lizarrondo]

Replying to fredck:

You have done a great job Alfonso.

In my previous thoughts about it, I've been thinking about creating a new command called "QuickUpload" for this kind of upload, instead of reusing "FileUpload", which would be reserved for the File Browser. Maybe this separation could bring us ways to handle the upload accordingly, by, for example, handling special configurations for the quick upload. Of course, the upload code would be shared between those commands.

The QuickUpload isn't sending right now any command, so we could name it anyway we like, but due to the fact that it isn't used right now I would wait until we got a clear idea about what should be changed before creating a new command that doesn't provide any new feature.

One of the reasons for the command separation it that I was wondering that, for backward compatibility, shouldn't we provide a configuration similar to "UseFileType". So when upgrading, our users will not have many surprises.

That's why I've added the ConfigFileTypesPath setting: If an installation was using a single folder then it's easy to change that setting for each file type and everything goes as usual, but if they use both the file browser and the quickupload they have the 4 folders created and they can go on knowing that now if they change the folder for upload or browsing that setting will be also used without any other special coding.

[Alfonso Martínez de Lizarrondo]

Replying to fredck:

Replying to alfonsoml:

Theres a new setting in the config file so the user can choose the possible commands enabled for the connector (for example only browse, but not upload or viceversa)

I would avoid this feature for now because its usage could bring confusion to the end user, as the interface would not reflect the configured settings.

As I said the aim is to be able to let the developer make it so the users can keep uploading files, but not browsing the server like they had with the 2 connectors, and also now being able to browse but not upload anything, the other options are there because I thought that it was better to do it in a generic way instead of hardcoding a boolean setting only to enable/disable the file browsing. If someone wants to disable other options from the connector he can manually adjust the filebrowser html files as well.

There's a new setting to easily set where should each file type go

This one is cool, as it has been strongly requested previously.

I liked the way you have proposed it initially, by concatenating the "configured base path" with the type path (like ConfigUserFilesPath & "file/" in ASP). But, looking at the sources, you have changed it to "/userfiles/files/", and repeated it for each type.

I would prefer the initial proposal, as it brings a single point of configuration, which would be ok for the majority of users. Then, for those who really need more control for each type, they could go to the specific type and make the changes.

Ok, initially I thought that way, but then I had an urge to do a clean up and keep everything simpler, but it can be done one way or another, there's no problem and I'll revert to the original way in the next checkin

[Frederico Caldeira Knabben]

Replying to alfonsoml:

Replying to fredck:

You have done a great job Alfonso.

In my previous thoughts about it, I've been thinking about creating a new command called "QuickUpload" for this kind of upload, instead of reusing "FileUpload", which would be reserved for the File Browser. Maybe this separation could bring us ways to handle the upload accordingly, by, for example, handling special configurations for the quick upload. Of course, the upload code would be shared between those commands.

The QuickUpload isn't sending right now any command, so we could name it anyway we like, but due to the fact that it isn't used right now I would wait until we got a clear idea about what should be changed before creating a new command that doesn't provide any new feature.

Well, that is not exactly true an in "upload.php" you are defining the command which will handle the quick upload (FileUpload).

Second point: there is a substantial difference between the normal upload and the quick upload. In the first case, you will be uploading the file in the "current folder". In the quick upload instead, you will be sending the file to a predefined folder, which can also be out of the File Browser space (previously, by setting UseFileType=false).

Many people prefer to have the Quick Upload pointing outside the File Browser to not mix the well organized files structure of the File Browser, with the "throw anything can" of the quick upload.

So, I believe it is better to handle the quick upload with a dedicated command (QuickUpload), which of course shares the FileUpload code.

Considering the above, maybe it is also a nice idea to have a dedicated setting for it:

$Config['QuickUploadPath']['File'] = '/userfiles/' ;

[Alfonso Martínez de Lizarrondo]

Replying to fredck:

Replying to alfonsoml:

Replying to fredck:

You have done a great job Alfonso.

In my previous thoughts about it, I've been thinking about creating a new command called "QuickUpload" for this kind of upload, instead of reusing "FileUpload", which would be reserved for the File Browser. Maybe this separation could bring us ways to handle the upload accordingly, by, for example, handling special configurations for the quick upload. Of course, the upload code would be shared between those commands.

The QuickUpload isn't sending right now any command, so we could name it anyway we like, but due to the fact that it isn't used right now I would wait until we got a clear idea about what should be changed before creating a new command that doesn't provide any new feature.

Well, that is not exactly true an in "upload.php" you are defining the command which will handle the quick upload (FileUpload).

Second point: there is a substantial difference between the normal upload and the quick upload. In the first case, you will be uploading the file in the "current folder". In the quick upload instead, you will be sending the file to a predefined folder, which can also be out of the File Browser space (previously, by setting UseFileType=false).

Many people prefer to have the Quick Upload pointing outside the File Browser to not mix the well organized files structure of the File Browser, with the "throw anything can" of the quick upload.

So, I believe it is better to handle the quick upload with a dedicated command (QuickUpload), which of course shares the FileUpload code.

Considering the above, maybe it is also a nice idea to have a dedicated setting for it:

$Config['QuickUploadPath']['File'] = '/userfiles/' ;

I defined the "FileUpload" command in the upload files to have the code as most similar as possible to the connector file, and that way check with the same code if the uploading of files is enabled or not. In comment 14 I meant that the Html form isn't sending any command right now, and if the behavior of both commands is gonna be the same then we could avoid adding it until there's a need that justify it.

I didn't thought that there were people that would rather have the quickuploaded files in a folder outside the browser, I only thought that there were two user cases:

• Don't allow to browse the server, so the files are uploaded to a common directory and there's no need for the subfolders
• The file browser is enabled, if a file is quick uploaded it should be possible to browse it at a later time.

Do you think that if we enable by default (or properly document how to do it) to a SUBfolder with just the FCKConfig.LinkUploadURL setting should be enough for those people?
I mean: the new code should be able to handle the CurrentFolder parameter in the quick upload, so the LinkUploadURL can be set to upload to a folder "/uploads/" or the like, without adding another configuration in the connectors themselves.

I'm suggesting this approach because in PHP adding a Config['QuickUploadPath'] setting for each type will need another setting also for the mapped folder: Config['QuickUploadAbsolutePath'], and that's 8 settings, and the code to get the current folder now will have to take care of the issued command to get it right.

[Frederico Caldeira Knabben]

Replying to alfonsoml:

I didn't thought that there were people that would rather have the quickuploaded files in a folder outside the browser

Just to exemplify, I've saw cases where the folder for quick upload had hundreds of images, while the File Browser was used only for well organized enterprise docs, logos, etc...

Other than that, we should be backward compatible "by default", so the quick upload should send the files to "/usefiles/", while the File Browser will be working at "/userfiles/file/" and so on. Drastic changes can be thought for version 3.0, but not in a minor version.

Do you think that if we enable by default (or properly document how to do it) to a SUBfolder with just the FCKConfig.LinkUploadURL setting should be enough for those people?
I mean: the new code should be able to handle the CurrentFolder parameter in the quick upload, so the LinkUploadURL can be set to upload to a folder "/uploads/" or the like, without adding another configuration in the connectors themselves.

This is an error we've done in the past and we should not repeat it anymore: no configurations must go in the file browser URLs. Thinks must be done in the server, for security.

I'm suggesting this approach because in PHP adding a Config['QuickUploadPath'] setting for each type will need another setting also for the mapped folder: Config['QuickUploadAbsolutePath'], and that's 8 settings,

It is better to give full control to developers. It will not be a big issue, as we will have the main variable (/userfiles/) which can be used to easily change the base path for all settings. Then, for those few developers who need more precise control, they can go to the fine grained settings.

and the code to get the current folder now will have to take care of the issued command to get it right.

Your affirmation justifies the need of the "QuickUpload" command.

[Alfonso Martínez de Lizarrondo]

I've added the QuickUploadPath setting and so I had to change all the places where a call to ServerMapFolder or similar was done to also specify the current command.

Just to clarify something about the previous comment: I was suggesting to use the current CurrentFolder parameter, that shouldn't bring any new security issues because that parameter is already used for the file browser commands, and the only difference would be that the quickuploaded files would end up in \userfiles\file\upload\ without any other change to the code, because due to the changes that I did it does already work for the quickupload as it does for the normal upload.

[Martin Kou]

The following SF report has been market as duplicate of this: ​http://sourceforge.net/tracker/index.php?func=detail&aid=1254053&group_id=75348&atid=543656

[Alfonso Martínez de Lizarrondo]

Fixed on trunk with [413]

I'll leave this open until I update the wiki and make sure that everything is fine

[Alfonso Martínez de Lizarrondo]

I've updated the wiki, closed all the related bugs that I could find and disabled by default the connectors, I think that now this is over.

[Thomas Tallyce]

Other than that, we should be backward compatible "by default", so the quick upload should send the files to "/usefiles/", while the File Browser will be working at "/userfiles/file/" and so on. Drastic changes can be thought for version 3.0, but not in a minor version.

I think what is most important is actually to get things fully correct, even though this is a point version - there seem to have been so many problems with the legacy connectors, which your work seems now to have fixed, that it would be good for people to have to take the opportunity to configure them well.

[Thomas Tallyce]

I haven't had a chance to download the new files, but on the face of it this seems like a very extensive set of extremely useful work - well done, and very many thanks.

Is there any possibility to get in the regexp feature noted at ​http://dev.fckeditor.net/ticket/306 before these connectors get released? Certainly in the PHP case that functionality is fairly easy to implement - just search for 'regexp' in ​http://dev.fckeditor.net/attachment/ticket/306/phpconnector-diff-v241-v1-tallyce.txt and you'll see how it's relatively easy to implement.

(Obviously I would also be keen to see some configurability of what happens when a file of the same name is uploaded, as per ticket #306 again, but I realise that's probably more specialised. However I do think regexp validation is a sufficiently generic requirement.)

[Alfonso Martínez de Lizarrondo]

That regexp would be very problematic, it means that as soon as a user tries to upload a file with a character no in the regexp the upload would fail and the user will wonder what's going on.

It's much better to do it in the way that it's proposed in ​https://sourceforge.net/tracker/index.php?func=detail&aid=1209255&group_id=75348&atid=543655 but even then I don't think that's the ideal solution because it means that the uploaded files may have lots of underscores depending on the users locale.

Anyway that's outside the scope of this patch.

[Thomas Tallyce]

That regexp would be very problematic, it means that as soon as a user tries to upload a file with a character no in the regexp the upload would fail and the user will wonder what's going on.

I think the original patch I proposed in ticket #306 could include an additional error code so that the user knows there is a filename mismatch.

But, aside from the issue of the specific regexp I suggested, could I again ask that regexp checking be a supported option, defaulting to false (or an empty string)? Again, I do think this should be relatively easy to implement, as the patch submitted demonstrates. It's very similar to the file extension checking mechanism.

(I've reopened this ticket, just so the suggestion isn't lost.)

[Alfonso Martínez de Lizarrondo]

This patch is about unifying the connectors and the auxiliary changes so everybody could keep on using them the way they did.

Any work on a regexp will be done in ​https://sourceforge.net/tracker/index.php?func=detail&aid=1209255&group_id=75348&atid=543655 or in another tracker, not here.