[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: svn commit: r1547866 - /subversion/trunk/subversion/libsvn_fs_fs/rep-cache.c

From: Stefan Sperling <stsp_at_elego.de>
Date: Wed, 4 Dec 2013 19:36:57 +0100

On Wed, Dec 04, 2013 at 06:16:40PM -0000, philip_at_apache.org wrote:
> Author: philip
> Date: Wed Dec 4 18:16:40 2013
> New Revision: 1547866
>
> URL: http://svn.apache.org/r1547866
> Log:
> Fix issue 3437, rep-cache.db created with wrong permissions.
>
> * subversion/libsvn_fs_fs/rep-cache.c
> (open_rep_cache): Create file before calling into SQLite.

Hey Philip,

I was just about to commit this patch and log message.
Do you see anything you like in here?

[[[
Fix issue #3437, "rep-cache.db created without group write bit".

Subversion now honours the umask when creating new rep-cache.db databases,
both during 'svnadmin create' as well as during normal operation (in which
case the umask of the server process is used).

Note that 'svnadmin create' didn't create a rep-cache.db previously.
But I don't see any problem with making it do that for purposes of
file permission consistency.

* subversion/include/private/svn_io_private.h
  (svn_io__merge_default_file_perms): Declare.

* subversion/libsvn_fs_fs/fs_fs.c
  (svn_fs_fs__create): Create rep-cache.db during FSFS creation so that
   permissions are set up in accordance with the umask of the 'svnadmin
   create' process.

* subversion/libsvn_fs_fs/rep-cache.c
  (open_rep_cache): If the rep-cache is newly created, tweak its permissions
   in accordance with the current umask. An existance check is performed every
   time the rep-cache is first opened. I hope the overhead of an extra stat is
   acceptable. No change on Windows, since we don't deal with permissions
   there anyway.

* subversion/libsvn_subr/io.c
  (merge_default_file_perms): Rename to...
  (svn_io__merge_default_file_perms): ... this, to expose it in the private
   API namespace.
  (svn_io_open_unique_file3): Adjust caller of merge_default_file_perms().

]]]

Index: subversion/include/private/svn_io_private.h
===================================================================
--- subversion/include/private/svn_io_private.h (revision 1547845)
+++ subversion/include/private/svn_io_private.h (working copy)
@@ -109,5 +109,12 @@ svn_stream__aprfile(svn_stream_t *stream);
 }
 #endif /* __cplusplus */
 
+/* OR together permission bits of the file FD and the default permissions
+ of a file as determined by get_default_file_perms(). Do temporary
+ allocations in SCRATCH_POOL. */
+svn_error_t *
+svn_io__merge_default_file_perms(apr_file_t *fd, apr_fileperms_t *perms,
+ apr_pool_t *scratch_pool);
 
+
 #endif /* SVN_IO_PRIVATE_H */
Index: subversion/libsvn_fs_fs/fs_fs.c
===================================================================
--- subversion/libsvn_fs_fs/fs_fs.c (revision 1547845)
+++ subversion/libsvn_fs_fs/fs_fs.c (working copy)
@@ -1277,6 +1277,17 @@ svn_fs_fs__create(svn_fs_t *fs,
                                  pool));
     }
 
+ /* Issue #3437: Create a rep-cache.db if rep-caching is supported,
+ * and set its access permissions in accordance with the umask.
+ * Sqlite uses mode 644 in most situations, and ignores the umask.
+ *
+ * We do this at FS creation time since it would imply an extra stat
+ * performance hit during regular operation. There is no way of knowing
+ * if rep-cache.db was newly created after calling svn_sqlite__open().
+ */
+ if (format >= SVN_FS_FS__MIN_REP_SHARING_FORMAT)
+ SVN_ERR(svn_fs_fs__open_rep_cache(fs, pool));
+
   /* This filesystem is ready. Stamp it with a format number. */
   SVN_ERR(svn_fs_fs__write_format(fs, FALSE, pool));
 
Index: subversion/libsvn_fs_fs/rep-cache.c
===================================================================
--- subversion/libsvn_fs_fs/rep-cache.c (revision 1547845)
+++ subversion/libsvn_fs_fs/rep-cache.c (working copy)
@@ -79,7 +79,12 @@ open_rep_cache(void *baton,
   svn_sqlite__db_t *sdb;
   const char *db_path;
   int version;
+#ifndef WIN32
+ svn_boolean_t rep_cache_existed;
 
+ SVN_ERR(svn_fs_fs__exists_rep_cache(&rep_cache_existed, fs, pool));
+#endif
+
   /* Open (or create) the sqlite database. It will be automatically
      closed when fs->pool is destoyed. */
   db_path = path_rep_cache_db(fs->path, pool);
@@ -100,6 +105,22 @@ open_rep_cache(void *baton,
      set it earlier. */
   ffd->rep_cache_db = sdb;
 
+#ifndef WIN32
+ if (!rep_cache_existed)
+ {
+ apr_file_t *db_fd;
+ apr_fileperms_t perms;
+
+ /* Issue 3437: When creating a new db, we must tweak access permissions
+ in accordance with the umask because sqlite doesn't do this. */
+ SVN_ERR(svn_io_file_open(&db_fd, db_path, APR_READ, APR_OS_DEFAULT,
+ pool));
+ SVN_ERR(svn_io__merge_default_file_perms(db_fd, &perms, pool));
+ SVN_ERR(apr_file_perms_set(db_path, perms));
+ SVN_ERR(svn_io_file_close(db_fd, pool));
+ }
+#endif
+
   return SVN_NO_ERROR;
 }
 
Index: subversion/libsvn_subr/io.c
===================================================================
--- subversion/libsvn_subr/io.c (revision 1547845)
+++ subversion/libsvn_subr/io.c (working copy)
@@ -1533,9 +1533,9 @@ get_default_file_perms(apr_fileperms_t *perms, apr
 /* OR together permission bits of the file FD and the default permissions
    of a file as determined by get_default_file_perms(). Do temporary
    allocations in SCRATCH_POOL. */
-static svn_error_t *
-merge_default_file_perms(apr_file_t *fd, apr_fileperms_t *perms,
- apr_pool_t *scratch_pool)
+svn_error_t *
+svn_io__merge_default_file_perms(apr_file_t *fd, apr_fileperms_t *perms,
+ apr_pool_t *scratch_pool)
 {
   apr_finfo_t finfo;
   apr_fileperms_t default_perms;
@@ -4993,7 +4993,7 @@ svn_io_open_unique_file3(apr_file_t **file,
    * case, but only if the umask allows it. */
   if (!using_system_temp_dir)
     {
- SVN_ERR(merge_default_file_perms(tempfile, &perms, scratch_pool));
+ SVN_ERR(svn_io__merge_default_file_perms(tempfile, &perms, scratch_pool));
       SVN_ERR(file_perms_set2(tempfile, perms, scratch_pool));
     }
 #endif
Received on 2013-12-04 19:37:39 CET

This is an archived mail posted to the Subversion Dev mailing list.