Why tempDir is created by this package and not by ufs-local package?

[rodrigok]

There are a lot of adapters that don't need a temp file. Or it can be optional.

[jalik]

It's mainly for performance reason, as it receives a chunk, the store writes it directly on the local file system which is the fastest way to save data and then get the next chunk, instead of establishing a network connection, then forwarding the chunk (I am thinking about cloud storage).

But it also provide an easy way to cancel or remove the file if something went wrong during transfer.

It is also secured in a way that it not written to the store until the file has been completely uploaded.

I really don't think that using a temp file has more bads than goods, so I'll keep it, but if someone shows me a better way, I can remove it later, this won't change a lot of things in the API.

[rodrigok]

I understand, but we can have some another temp options, like MongoDB as a temp store.

Can exists cases when user don't have permission to write to FileSystem with the user that is running the application.

Can exist a temp in memory too. Probably in this case more options is Better.

[jalik]

@rodrigok, a temp in memory is just the worst idea you could have when creating a temporary file, we are not on a user's desktop, we're talking about servers with potentially thousands of user requests per seconds, a single user could easily upload a 10Mo file, but what if 100 users did that at the same time, 1Go of RAM ? no way... say hello to outOfMemoryException and good bye to service availability (but maybe I misunderstand your idea, so tell me).

For a Mongo temp storage, why not, but this would involve using GridFS, because a document in Mongo is limited to 16Mo and I see this as a duplicate of the work you are currently doing on the ufs-gridfs store.

But you're right, there is a possibility that some developers do not have write permissions on a private local directory, however this concerns IMO a very small portion of people, as nowadays even on a shared hosting, you have an access to a "home" dir that is not accessible from Internet.

Now, as I said, until someone tells me that this is a problem for him because he has no write permissions, I don't see this feature as a priority, so it will be delayed for future releases as a "bonus".

Just to know, are you facing this problem of write access ?

[rodrigok]

Hei @jalik, you are right, this is not a priority now, and I'm not facing this problem now.

I understand all problems generated by keeping file in memory during upload, however use FS to store the file before send to GridFS seems a lack of performance with no necessity.

If user don't have a server with SSD this solution can be very slow in same cases.

My ideia isn't remove the temp store, but make it optional and enabled by default, so users can disable to improve performance or solve access problems in some cases.

But, as you said, isn't a problem now, but I think it is a good issue to resolve in a near future.

[ericvrp]

Today I installed Rocket.chat (which uses your Meteor package) with mup on a DigitalOcean server. These are the temp directory permissions: drwxr-xr-x 2 root root 4096 Jan 6 03:59 ufs Rocket.Chat runs as meteoruser so could not write here. So not sure if this qualifies as a Rocket.chat or jalik-ufs bug but an in-memory/mongetemp default would have prevented this.

For those needing to fix the /tmp/ufs permissions in cases similar to mine:

su - cd /tmp chown meteoruser:meteoruser ufs chmod og-rwx ufs

[jalik]

@ericvrp Hi, as the UFS doc says, you can define your own tmpDir path, it can be wherever you want, so you could even point to a folder in your home dir where you have the permissions :

// Set the temporary directory where uploading files will be saved
// before sent to the store.
UploadFS.config.tmpDir = '/tmp/uploads';

The problem for now, is to define this config after RocketChat, otherwise it will be overwritten. I would like to solve this quickly but I don't have lot of time for such improvements (let's say it's not a bug).

Quick reminder : InMemory storage is a bad idea as I said in a previous post, we're talking about uploading files, potentially hundred of MB of data...

Edit : Actually, you would just have to set the permissions to RWX and the owner to the user that runs your Meteor app on the tmpDir.

[ericvrp]

Yes I found that line in the docs, which made me look for it in RocketChat but it's not there so either I looked not at the right place or it relies on the default. In any case /tmp/ufs had the wrong filesystem permissions. It would be good to know why.

I agree with the worst case scenario for inmemory temp storage but it has advantages too. One option could for instance be to allow a max amount of memory usage before a. falling back to disk or b. pausing the upload until below the memory threshold again.

One could ofcourse already store inmemory by using a ramdisk. for the /tmp directory.

BTW at this stage not to important anyway probably. I have also posted my findings on a RocketChat channel because perhaps the /tmp/ufs permission is something they can fix or provide better error messages for.

[pavelpep]

When using mup deploy, the app is started as root and the temp dir created is owned by root,
with permissions rwx r-x r-x. However, when the app steps down to meteoruser, it no longer has permissions to write in the temp folder.

Please add a config option to specify permissions for the temp dir and/or specify which user owns the folder.

[jalik]

@pavelpep There is no need to add such an option since the current permission is the one needed to make it work, don't you agree that the user that created the temp folder should be the only allowed to write into it ?

It's not that I don't want to help but you want to fix this issue in the wrong way giving more permissions than needed on a temp folder.

In your case, the problem is with MUP, why is it starting your app as root ? it's insecure, you should check your config if you can force MUP to execute the app only with the meteor user.

Also, you can create yourself the folder and set ower/permissions before you start the app.

[pavelpep]

@jalik Thanks for the quick reply! The second work around you suggested (create the folder manually) is what we are currently using, but it would be much cleaner if there was a way to explicitly set the owner of the temp folder right in the UFS config.

I agree the user who created the folder should be the only one to write to it. But, it would also be useful limit 'others' from reading the contents. Currently, when we create the folder manually with permissions 740 , they get overridden to the library default 744 upon app start-up.

[jalik]

@pavelpep The ufs-local package has it's own documentation and issue tracker : https://github.com/jalik/jalik-ufs-local#creating-a-store

You can set folder's permissions passing mode: 740 and file's permissions using writeMode: 740 to new UploadFS.store.Local(options).

[pavelpep]

@jalik, I know, but what I am referring to above is strictly regarding the temporary folder and its permissions.

The ufs-local package already has the option to specify permissions for the data storage folder and files, I was just asking if you could implement similar options for the temporary folder created by this package.

[jalik]

@pavelpep Sorry I was focusing on the Local store, you can now define temp directory permissions by setting the UploadFS.config.tmpDirPermissions to a string like 0700. See https://github.com/jalik/jalik-ufs/tree/v0.6.4#configuration.

[pavelpep]

@jalik awesome, thank you :)