Question related to the preview/path and possible improvements

[fabriceci]

Hi, I am writing a connector for java (spring MVC). I tried the JSP connector in the repo but it's not working. (I discovered later the C5 connector but I have not tried).

To create the connector, I watched the Ajax call and response, to try to understand.

My app doesn’t serve the files directly to the client, I have 2 path :

• one private (only accessible by the server), used to display file in the manager => « Path »
• one public (to access to the file), used to browse the file => « Preview »

(When you try to listen a song or watch a doc the manager use the preview path (and thumbnail for images), so I think it’s the good behaviour, I am right?

The response look like this

{  
...
"Path":"/my_folder/fichier.txt",
"Preview" : "/../../../fileController?file=public/my_folder/fichier.txt", 
...
}

The connector work well but I have 2 behaviour I would like to change (but maybe i miss something):

1. When I select a file (editor) or copy URL to clipboard I have the (server) path, and I would like the preview.

Do you agree if I add a pull request to add a boolean in the config file to choose that something like "usePreviewForSelectionAndCopy = false" ?

2. For the preview, the manager add automatically the path of the file manager directory ( that’s why I was forced to begin the preview path with "/../../.."). I think the issue related to the clipboard is maybe linked to this choice.

Same, do you agree if I add pull request to add a boolean in the config to choose if the preview path should be relative to the manager or absolute: something like « absoluteUrlForPreview = false »?

[psolom]

Hi, lets try to be consistent.
I try to understand the whole thing, but the description is a bit confusing.

1. So you are writing MVC connector, right? I had a discussion with another guy who had written Java connector, but for the old repo. I have asked him to create PR and maintain that connector and I am waiting for the feedback currently. I just want to be sure you both are not going to work on the same thing.

2. Can you be more precise at your particular case?
   First of all, which way you are using filemanager that you need "private" path to be returned? I'm not sure, but I can assume that you are talking about the case using FM for some WYSIWYG to select and put some files to an editor. If so, I haven't tested my FM version with any of WYSIWYG editors to be honest, and need to investigate this. Correct me if I am wrong with your case.

3. Which API call you suggest to return such kind of response? I can see "getinfo" response contains similar data. Both "path" and "preview" params contains relative path. For example, when I open some image or audio in FM I get something like that:

{
...
Path: "/audio.mp3"
Preview : "/userfiles/audio.mp3"
...
}

Do you suggest to replace them as at your code snippet? Again, I need to understand you case in details.

4. Those config options you suggest to add looks too specific. Lets deal with your case and, perhaps, we could fine more elegant solution. Any additional details are welcome.

[fabriceci]

1

Yes, I wrote a connector for JAVA running with the Spring MVC framework ( /!\ Your hyperlink on the word MVC leads to a connector in Microsoft .NET MVC language). In the current repo, you have a JSP connector (JAVA) but it doesn't work.

2

I use the filemanager for manage files on the server and to select an image in the WYSIWYG editor (tinymce). Nothing special.

I will take your example with the getinfo API response to explain my case.

{ ... Path: "/audio.mp3" Preview : "/userfiles/audio.mp3" ... }

So the file is /audio.mp3.

In java, we usually upload the file on an external folder outside the app, in my app this file will be stored for example in this folder : **/srv/webdata/appName/upload/**audio.mp3

But you can’t access to this path directly in the browser, you must pass through a controller; for example, to view this file we need to go in this address : http://www.site.com/public/audio.mp3

Short summary :

• real path on the server : /srv/webdata/appName/upload/audio.mp3
• path to browse the file : http://www.xxx.com/public/audio.mp3

In the getinfo API response, we have 2 main data : Path & Preview

In my connector, I use

• Path to find a file on the server (in java code) like the php connector and
• Preview as the relative path to view the file in the browser.

Does my use of field preview is correct?

(If the usage of preview is correct, there is no problem, I tell you that for information purposes.)

3

Yes, it was the getinfo API response. My API response is the same as the php connector.

In my app, as I use a controller to browse the file, so for the audio.mp3 example, my API response will be :

{ ... Path: "/audio.mp3", Preview : "../../../public/audio.mp3" ... }

As the preview path is relative to the filemanager path, I must set : "../../../public/audio.mp3" to lead to http://www.site.com/public/audio.mp3 )

It's bit ugly but It’s working well.

So (if all works well) why I asked to add 2 new parameters.

a) for the select function (in WYSIWYG) and COPY function.

(This point is independent of my case)

If the preview field is the correct path to view the file in the browser. When you copy the path into the clipboard or select an image in the WYSIWYG editor, you should have the preview's value (and not the path). See this screenshot (I took your audio.mp3 file for the example)

I think this issue is related to the same problem

b) absolute path for preview

Because in my case, the preview path should not depend on to the file manager path. ( or maybe sometimes, we would like to serve heavy files in a external website).

I've seen a possible solution in the S3Filemanager.php, instead of serve the file directly, use the GET param mode=readfile for the preview :

$item['Preview'] = $this->connector_script_url . '?mode=readfile&path=' . $relative_path;

but that implies to pass through a controller to serve the file, this will be bad for performance if you planned to serve static files through a reverse proxy like Nginx.

Now , maybe I miss a settings or misunderstood something, I hope it's more clear now.

[psolom]

Thank you for the detailed explanation.

I have got your case now and decided to check PHP with the TinyMce. As I said before, I made a lot of changes, but I didn't bother to check how it works in conjunction with any of WYSIWYG editors. That is why you faced with the issue that you have fixed your PR. The same thing with the "path" and "preview" params.

I have just tested FM in TinyMce and got some issues as well. I agree that an absolute path should be returned for WYSIWYG and COPY function. If I got you right all your problems can be solved in case the absolute path is treated for preview and inside WYSIWYG instead of relative one. Is that will be enough?

I'm not sure because you specified your path as http://www.site.com/public/audio.mp3, what is "public" folder? Is it the same as "userfiles" in the code snippet? Let's suppose so, then you will get http://www.site.com/userfiles/audio.mp3 for the code snippet example. Is that ok? Let me know if I understood the whole thing correct.

Anyway I am going to resolve the current problem keeping in mind your case.

[fabriceci]

If you agree for the full path for the functions "select" and "copy", I think we should consider in the API response, "Preview" as the reference value to be used to view/select or copy/paste) a file and keep "Path" value only for server exchanges.

For now in a file's detail, the file manager uses the "preview" value to view files except for images, they use the value of "Thumbnail". Images should also use "Preview" value (to keep the same logic and because in any case we need the preview value for the function "select" and "copy").

What do you think about that?

Even if you/we fix the functions above, I will still have my ugly path if I don't want to pass through a controller to get the file :

// current function
var createPreviewUrl = function(path) {
   return location.origin + location.pathname + path;
}

// in my case result will be: => http://mysite.com/folder/of/plugin/../../../public/myfile.mp3 

I think I found an elegant solution, it will allow to add these functionalities (simply) without break the existing behaviour.

var createPreviewUrl = function(path) { 
 //  = the path is already an absolute path
if(path.substr(0,4) === 'http' || path.substr(0,3) === 'ftp')   return path;

// normalizePath is a new function
return location.origin + normalizePath(location.pathname + path);
};

Suppose that location.origin + location.pathname = "http://mysite.com/folder/of/plugin"

For the API response "getinfo", here is 4 possibility of values for "Preview" :

case 1 => "/userfiles/audio.mp3"; // relative path (folder bellow fm's root)
case 2 => "?mode=readfile&path=/userfile/audio.mp3"; // use the connector the get the file
case 3 => "/../../../public/audio.mp3"; // relative path (folder above fm's root)
case 4 => "http://otherwebsite/path/audio.mp3"; // use an external website/ftp or an absolute path

case 1 and case 2 => same behaviour as before

case 3 :
Previous function return :  http://mysite.com/folder/of/plugin/../../../public/audio.mp3 
Now : http://mysite.com/public/audio.mp3    => path is now clean

case 4
Previous function return : http://mysite.com/folder/of/pluginlhttp://otherwebsite/path/audio.mp3
Now : http://otherwebsite/path/audio.mp3 => supported

There is no a lot of changes to do, here is the PR, It will easier to understand.

[psolom]

Thank you. I will take a look this tomorrow.

[psolom]

I have accepted your PR, but as I mentioned, there were some issues for specific cases. You can face it if open file in preview mode which are not supposed to be previewed. For instance *.html or *.zip. Those file types don't have appropriate viewer associated by default. In this case preview page should display placeholder icon. And there were other settings-specific cases. Anyway the idea behind your PR is good, so I have developed it and made some bugfix and modifications.

You can see not "getinfo" method returns following pathes:

• 'Path' - For internal FM usage only (no changes here)
• 'Dynamic Path' - Path to files prefixed with root folder, for preview usage at the client-side (you were named it as "Preview")
• 'Image Path' - Path to original image OR thumbnail OR placeholder icon

Also I have added new outsideServerRoot option in config file. 'Dynamic Path' and 'Image Path' params return a relative path mostly, to build an absolute path at the client-side. But there are cases when you can reach a file solely by making a request to a connector. For example, you can set it to "true" if you have the userfiles folder located beyond server root directory to be able to preview images and media files.

I'm going to describe all the thing in Wiki as well. Hope you will be good with my modifications.

[fabriceci]

Yes, you are right, I forgot to test this kind of file, my bad!

That's perfect, the connector can now choose the way it wants to serve files!

Just a few comments

• Extra or => Line 525 filemanager.js :

// already an absolute path or or a relative path to connectors action

• Rename "Dynamic Path" to "File Path". (I don't understand why dynamic).
• I did not understand the parameter outsideServerRoot directly, maybe it will help to rename the parameter to something like "previewFilesThroughConnector" . However, I wonder if this setting is really relevant, maybe we should delete it, the connector should choose the path to preview a file according his implementation. What do you think?

[psolom]

1. Yeah, typo.

2. I have it named "dynamic" at PHP connector, so that is it. Actually I don't mind of "File Path" instead of "Dynamic Path", or some other appropriate name. We can name it back with "Preview Path" as well. I just want to name all those 3 path-related params in a way to make their purpose clear. Suggestions are welcome.

3. Generally you are right. I also thought about a way to define preview path in a connector directly, but I had a lack of time and came to quick client-side solution. Not comprehensive though. Now I have moved it to the connector and removed outsideServerRoot option from config file. Check it please.

Thanks for your notes.

[fabriceci]

In my opinion, the big advantage of the plugin is the simple API, it's easy to create a connector (and there are already many connectors available even is it should be updated).

I think you should keep the API the more simplest as possible and avoid changes that would break connectors.

So I suggest that :

'Path' - For internal FM usage only (no changes here)
'Dynamic Path' - Path to files prefixed with root folder, for preview usage at the client-side (you were named it as "Preview")
'Image Path' - Path to original image OR thumbnail OR placeholder icon

become

'Path' - For the FM usage only 
'Preview' - Path to preview a file  => All files , even images (for now for the preview of an image, you use the Image/Thumbnail path)

'Thumbnail' - Path to the thumbnail of the file

Names are the same as before, they were good (and avoid breaking connectors).
But thumbnail Image Path) should return only the path of the thumbnail (and not the preview path if the file is an image).

See my PR, after this, I think we are done with this issue.

[psolom]

I have accepted your PR with a note and commited minor adjustments. You can close that issue if we have handled all things related to this particular issue.

[gmkll]

Hi,

I am in the process of merging this change into PR #21.

Doing this I could not get to work case 2) which is what I have to to

createPreviewUrl("/../../../public/audio.mp3") // relative path (folder above fm's root)
would result in http://mysite.com/public/audio.mp3, IF the code would be
location.origin + normalizePath(location.pathname + path); BUT it isn´t.

The code in current filemanager.js is different, as createPreviewUrl merges baseUrl and normalized path, so location.pathName would NOT be normalized as it is already in baseUrl.

Possible solution OK:
normalizePath(baseUrl + "/../../public/audio.mp3");
normalizePath(baseUrl + trimSlashes("/../../public/audio.mp3"));

BUT there is another bug: An absolute URL, e.g.
case 4 => "http://otherwebsite/path/audio.mp3";
is NOT preserved using the normalizePath function results in
"http:/otherwebsite/path/audio.mp3" (missing slash!).

In the original code it´s ok, as the scheme is preserved.
I'll try to fix this in PR #21 as well.

[fabriceci]

I was unable to test this case like I said in PR #20, I don't understand well the purpose of the "baseUrl" parameter. If you can give me some explanations and a use case, I can maybe help to solve this.

[gmkll]

baseUrl is an absolute url by default (location.origin + location.pathname), as you said in your comment and occasionally the connector part is provided for the thumbnail, which should be appended. This should give you case 2.
As it´s an absolute URL (and it may be also by an config.option.baseurl setting), normalizePath should be able to handle a scheme prefix, shouldn´t it?

Sorry, the better reason is a relative preview provided by configuration, i.e. a ressource which is relative to the webapp, and it´s path may contain "..".

[psolom]

Yeah, I have also noticed that normalizePath() function strips the second slash after the protocol name. If you need to put an absolute path to the normalizePath() function you are free to modify it, no problem. Just be sure you have tested it well.