Skip to content

Option to set cache file permissions and cache directory permission fix. #115

Closed
wants to merge 2 commits into from

2 participants

@jandreasn

Added option to set cache file permissions (like you can do with Zend_Cache_Backend_File). It's needed in some setups like ours.

The cache directory wasn't created with the set permissions under some conditions (probably an umask related issue), so added a fix for that as well.

@bobthecow
Owner

There's definitely an issue there, but this might be taking it in the wrong direction :)

If the system were properly configured, the current umask value would be appropriate. Working around a misconfigured umask in a library seems like a bad idea. If this library is a good citizen, it would write its files as the apache/php user, and apply the currently configured umask. So I'd rather change to something like this:

<?php
chmod($fileName, 0666 & ~umask());

This does put the onus on the user (or sysadmin) to set up their umask properly, but it should just work almost everywhere.

Is there a reason you're unable to fix your system settings?

@jandreasn

Thanks for considering :)

I'm not that familiar with unix systems/umasks and so on, so I might be wrong. But, for example, in a shared host environment the users might not be able to set the umask / or have to knowledge to set the umask.

Our case: We deploy to many different servers and environments, and don't really know about the umask setting on these. The mustache cache directory and files were created as the apache/php user, with 0644 permissions. The implicaiton of this was that the SSH/FTP user (not apache) didn't have any access to these files, and prevented any future change in the project file structure.

My opinion is that this should be optionally independent from any umask setting. Maybe people don't want all their files to be created with write permissions, just the mustache cache files. The Zend Framework user base is quite large, and they felt the need for this setting for their cache files. If you feel strongly about respecting the umask, maybe you could make the option an override instead, defaulting to umask if not set?

@bobthecow
Owner

Thanks. I do see the value in letting people override it, but I have a strong preference that they configure their system properly instead :)

I've merged your changes, with a couple of changes:

  • I called the option cache_file_mode to match the terminology used in PHP documentation when talking about file modes.
  • I did not merge the cache directory permission overrides. Creation of the cache directory can and should be part of app deployment. As a courtesy, Mustache tries to create the directory if it doesn't exist, but I don't want to override a system's settings in doing so. If you need different permissions for your cache directory, you are free to create and set up that directory yourself :)
  • I changed the default to use the current umask, and added documentation regarding that recommendation.

042d537...2e97db3

@bobthecow
Owner

As I said, this has been merged into dev and will go into the next release. Thanks again!

@bobthecow bobthecow closed this Nov 29, 2012
@jandreasn

Fair enough :) Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.