815541 - If data/params is a symlink, the symlink is removed and replaced by a text file the next time params are edited

Status: NEW

(Bugzilla :: Administration, task)

Description

[H.-Dirk Schmitt]

User Agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:17.0) Gecko/17.0 Firefox/17.0
Build ID: 20121120062532

Steps to reproduce:

Case 1: symlink data/params to a file under subversion crontrol
Case 2: make data/params readonly (chmod a-r), change something with "editparams.cgi"


Actual results:

Case 1: the symlink is removed and the content is restored in a normal file
Case 2: Changes are written to data/params and the file permissions are restored to ug=rw 


Expected results:

data/params should be handled as a "normal" file. 
* no remove of the file should be done
* permissions shouldn't be changed

Rationale:
======

Case 1: data/params is a typical configuration file, which can (should) be stored in a revision control system like subversion. This can be easily done with symlinks.

Case 2: In some cases it is useful to "lock" the configuration. 
E.g if data/params is generated in a cloned installation with a different url_base.

Comment 1

[Dave Miller [:justdave]]

There is a race condition while making changes to that file.  Removing the file can cause Bugzilla to crash if someone accesses the site while the changes are being written out, if we edit the file in place, because the webserver could try to load the file after we've truncated it and before the new contents are written.  The only safe way to change it is to write it out to a temporary file, then rename it overtop of the original after we're done writing it.  That way the web server will still have the filehandle to the old one open through the end of its current request being handled, and subsequent requests will open the new freshly-written copy.

Hence the "recreated on each change" is entirely on purpose, and there's no real safe way to change that.

You may be able to prevent changes to it by preventing the webserver from writing to the data directory itself.  This way it could neither create a new file or overwrite the old one (because the rename it does is effectively writing to the directory entry).  I believe all of the places it actually stores other temporary data are in further subdirectories of data rather than directly in the data directory, so as long as you leave the permissions on those additional subdirectories alone you'll probably be okay.

You might also look into a tool like puppet for deploying the config...  you can place the file in an arbitrary location which you have under version control and have puppet use that to replace the one in Bugzilla's directory if it's been changed.  You could also use rsync on a cron job to make sure the copy from your version control gets copied in.

Comment 2

[Dave Miller [:justdave]]

The only way I can think of to possibly deal with the symlink case is to explicitly check if it's a symlink and then resolve the symlink's target and directly overwrite the target file with our final rename. I suspect this would overly-complicate things for the common use case of people using the UI to edit the params, though.

Comment 3

[H.-Dirk Schmitt]

For the symlink case (1) the purpose is not only to "inject" a configuration from a revision control system.
If the configuration is updated the change should be vissible in the checkout of the revision control system.
E.g. 'svn diff' and 'svn commit' should be usable.

So a handling/preserving of a symlink would be great.
Similar preserving the rights would be also helpful and could be done in the same style. 

In my opinion a (non-binary) configuration file should always behave like a normal text file.

Comment 4

[Frédéric Buclin]

it doesn't make sense to write-protect data/params. The goal of editparams.cgi is to let admins set params. If you don't want admins to edit parameters, then remove their tweakparams privileges. So case 2 is wontfix. We override permissions on purpose.

About case 1, this can be seen as an enhancement.