subversion-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From br...@apache.org
Subject svn commit: r1854072 - in /subversion/trunk/subversion: libsvn_subr/io.c tests/libsvn_subr/io-test.c
Date Thu, 21 Feb 2019 18:04:16 GMT
Author: brane
Date: Thu Feb 21 18:04:15 2019
New Revision: 1854072

URL: http://svn.apache.org/viewvc?rev=1854072&view=rev
Log:
Fix issue #4806: Remove on-disk trees with read-only directories in them.

* subversion/libsvn_subr/io.c
  (io_set_perms): New; helper function for io_set_*_perms.
  (io_set_file_perms): Use io_set_perms.
  (io_set_dir_perms): New; like io_set_file_perms, but for directories.
  (io_set_readonly_flag): New; helper function for setting the read-only flag.
  (svn_io_set_file_read_only,
   svn_io_set_file_read_write): Use io_set_readonly_flag.
  (svn_io_remove_dir2): On Unix, make the parent directory writable before
   trying to remove its children.
  (svn_io_dir_remove_nonrecursive): On Windows, remove a directory's
   read-only flag before trying to remove the directory.

Modified:
    subversion/trunk/subversion/libsvn_subr/io.c
    subversion/trunk/subversion/tests/libsvn_subr/io-test.c

Modified: subversion/trunk/subversion/libsvn_subr/io.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/io.c?rev=1854072&r1=1854071&r2=1854072&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_subr/io.c (original)
+++ subversion/trunk/subversion/libsvn_subr/io.c Thu Feb 21 18:04:15 2019
@@ -1622,13 +1622,14 @@ merge_default_file_perms(apr_file_t *fd,
    that attempts to honor the users umask when dealing with
    permission changes.  It is a no-op when invoked on a symlink. */
 static svn_error_t *
-io_set_file_perms(const char *path,
-                  svn_boolean_t change_readwrite,
-                  svn_boolean_t enable_write,
-                  svn_boolean_t change_executable,
-                  svn_boolean_t executable,
-                  svn_boolean_t ignore_enoent,
-                  apr_pool_t *pool)
+io_set_perms(const char *path,
+             svn_boolean_t is_file,
+             svn_boolean_t change_readwrite,
+             svn_boolean_t enable_write,
+             svn_boolean_t change_executable,
+             svn_boolean_t executable,
+             svn_boolean_t ignore_enoent,
+             apr_pool_t *pool)
 {
   apr_status_t status;
   const char *path_apr;
@@ -1648,9 +1649,16 @@ io_set_file_perms(const char *path,
                             || SVN__APR_STATUS_IS_ENOTDIR(status)))
         return SVN_NO_ERROR;
       else if (status != APR_ENOTIMPL)
-        return svn_error_wrap_apr(status,
-                                  _("Can't change perms of file '%s'"),
-                                  svn_dirent_local_style(path, pool));
+        {
+          if (is_file)
+            return svn_error_wrap_apr(status,
+                                      _("Can't change perms of file '%s'"),
+                                      svn_dirent_local_style(path, pool));
+          else
+            return svn_error_wrap_apr(status,
+                                      _("Can't change perms of directory '%s'"),
+                                      svn_dirent_local_style(path, pool));
+        }
       return SVN_NO_ERROR;
     }
 
@@ -1750,10 +1758,50 @@ io_set_file_perms(const char *path,
       status = apr_file_attrs_set(path_apr, attrs, attrs_values, pool);
     }
 
-  return svn_error_wrap_apr(status,
-                            _("Can't change perms of file '%s'"),
-                            svn_dirent_local_style(path, pool));
+  if (is_file)
+    {
+      return svn_error_wrap_apr(status,
+                                _("Can't change perms of file '%s'"),
+                                svn_dirent_local_style(path, pool));
+    }
+  else
+    {
+      return svn_error_wrap_apr(status,
+                                _("Can't change perms of directory '%s'"),
+                                svn_dirent_local_style(path, pool));
+    }
 }
+
+static svn_error_t *
+io_set_file_perms(const char *path,
+                  svn_boolean_t change_readwrite,
+                  svn_boolean_t enable_write,
+                  svn_boolean_t change_executable,
+                  svn_boolean_t executable,
+                  svn_boolean_t ignore_enoent,
+                  apr_pool_t *pool)
+{
+  return svn_error_trace(io_set_perms(path, TRUE,
+                                      change_readwrite, enable_write,
+                                      change_executable, executable,
+                                      ignore_enoent, pool));
+}
+
+static svn_error_t *
+io_set_dir_perms(const char *path,
+                 svn_boolean_t change_readwrite,
+                 svn_boolean_t enable_write,
+                 svn_boolean_t change_executable,
+                 svn_boolean_t executable,
+                 svn_boolean_t ignore_enoent,
+                 apr_pool_t *pool)
+{
+  return svn_error_trace(io_set_perms(path, FALSE,
+                                      change_readwrite, enable_write,
+                                      change_executable, executable,
+                                      ignore_enoent, pool));
+}
+
 #endif /* !WIN32 && !__OS2__ */
 
 #ifdef WIN32
@@ -2115,6 +2163,55 @@ svn_io_set_file_read_write_carefully(con
   return svn_io_set_file_read_only(path, ignore_enoent, pool);
 }
 
+#if defined(WIN32) || defined(__OS2__)
+/* Helper for svn_io_set_file_read_* */
+static svn_error_t *
+io_set_readonly_flag(const char *path_apr, /* file-system path */
+                     const char *path,     /* UTF-8 path */
+                     svn_boolean_t set_flag,
+                     svn_boolean_t is_file,
+                     svn_boolean_t ignore_enoent,
+                     svn_boolean_t pool)
+{
+  apr_status_t status;
+
+  status = apr_file_attrs_set(path_apr,
+                              (set_flag ? APR_FILE_ATTR_READONLY : 0),
+                              APR_FILE_ATTR_READONLY,
+                              pool);
+
+  if (status && status != APR_ENOTIMPL)
+    if (!(ignore_enoent && (APR_STATUS_IS_ENOENT(status)
+                            || SVN__APR_STATUS_IS_ENOTDIR(status))))
+      {
+        if (is_file)
+          {
+            if (set_flag)
+              return svn_error_wrap_apr(status,
+                                        _("Can't set file '%s' read-only"),
+                                        svn_dirent_local_style(path, pool));
+            else
+              return svn_error_wrap_apr(status,
+                                        _("Can't set file '%s' read-write"),
+                                        svn_dirent_local_style(path, pool));
+          }
+        else
+          {
+            if (set_flag)
+              return svn_error_wrap_apr(status,
+                                        _("Can't set directory '%s' read-only"),
+                                        svn_dirent_local_style(path, pool));
+            else
+              return svn_error_wrap_apr(status,
+                                        _("Can't set directory '%s' read-write"),
+                                        svn_dirent_local_style(path, pool));
+          }
+      }
+  return SVN_NO_ERROR;
+}
+#endif
+
+
 svn_error_t *
 svn_io_set_file_read_only(const char *path,
                           svn_boolean_t ignore_enoent,
@@ -2126,24 +2223,11 @@ svn_io_set_file_read_only(const char *pa
   return io_set_file_perms(path, TRUE, FALSE, FALSE, FALSE,
                            ignore_enoent, pool);
 #else
-  apr_status_t status;
   const char *path_apr;
 
   SVN_ERR(cstring_from_utf8(&path_apr, path, pool));
-
-  status = apr_file_attrs_set(path_apr,
-                              APR_FILE_ATTR_READONLY,
-                              APR_FILE_ATTR_READONLY,
-                              pool);
-
-  if (status && status != APR_ENOTIMPL)
-    if (!(ignore_enoent && (APR_STATUS_IS_ENOENT(status)
-                            || SVN__APR_STATUS_IS_ENOTDIR(status))))
-      return svn_error_wrap_apr(status,
-                                _("Can't set file '%s' read-only"),
-                                svn_dirent_local_style(path, pool));
-
-  return SVN_NO_ERROR;
+  return io_set_readonly_flag(path_apr, path,
+                              TRUE, TRUE, ignore_enoent, pool);
 #endif
 }
 
@@ -2159,23 +2243,11 @@ svn_io_set_file_read_write(const char *p
   return io_set_file_perms(path, TRUE, TRUE, FALSE, FALSE,
                            ignore_enoent, pool);
 #else
-  apr_status_t status;
   const char *path_apr;
 
   SVN_ERR(cstring_from_utf8(&path_apr, path, pool));
-
-  status = apr_file_attrs_set(path_apr,
-                              0,
-                              APR_FILE_ATTR_READONLY,
-                              pool);
-
-  if (status && status != APR_ENOTIMPL)
-    if (!ignore_enoent || !APR_STATUS_IS_ENOENT(status))
-      return svn_error_wrap_apr(status,
-                                _("Can't set file '%s' read-write"),
-                                svn_dirent_local_style(path, pool));
-
-  return SVN_NO_ERROR;
+  return io_set_readonly_flag(path_apr, path,
+                              FALSE, TRUE, ignore_enoent, pool);
 #endif
 }
 
@@ -2761,6 +2833,12 @@ svn_io_remove_dir2(const char *path, svn
       return svn_error_trace(err);
     }
 
+  /* On Unix, nothing can be removed from a non-writable directory. */
+#if !defined(WIN32) && !defined(__OS2__)
+  SVN_ERR(io_set_dir_perms(path, TRUE, TRUE, FALSE, FALSE,
+                           ignore_enoent, pool));
+#endif
+
   for (hi = apr_hash_first(subpool, dirents); hi; hi = apr_hash_next(hi))
     {
       const char *name = apr_hash_this_key(hi);
@@ -4493,6 +4571,12 @@ svn_io_dir_remove_nonrecursive(const cha
 
   SVN_ERR(cstring_from_utf8(&dirname_apr, dirname, pool));
 
+  /* On Windows, a read-only directory cannot be removed. */
+#if defined(WIN32) || defined(__OS2__)
+  SVN_ERR(io_set_readonly_flag(dirname_apr, dirname,
+                               FALSE, FALSE, FALSE, pool));
+#endif
+
   status = apr_dir_remove(dirname_apr, pool);
 
 #ifdef WIN32

Modified: subversion/trunk/subversion/tests/libsvn_subr/io-test.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/libsvn_subr/io-test.c?rev=1854072&r1=1854071&r2=1854072&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/libsvn_subr/io-test.c (original)
+++ subversion/trunk/subversion/tests/libsvn_subr/io-test.c Thu Feb 21 18:04:15 2019
@@ -211,6 +211,38 @@ create_comparison_candidates(struct test
   return err;
 }
 
+/* Create an on-disk tree with optional read-only attributes on some
+   files and/or directories. */
+static svn_error_t *
+create_dir_tree(const char **dir_path,
+                const char *testname,
+                svn_boolean_t dir_readonly,
+                svn_boolean_t file_readonly,
+                apr_pool_t *pool)
+{
+  const char *test_dir_path;
+  const char *sub_dir_path;
+  const char *file_path;
+
+  SVN_ERR(svn_test_make_sandbox_dir(&test_dir_path, testname, pool));
+
+  sub_dir_path = svn_dirent_join(test_dir_path, "dir", pool);
+  SVN_ERR(svn_io_dir_make(sub_dir_path, APR_OS_DEFAULT, pool));
+
+  file_path = svn_dirent_join(sub_dir_path, "file", pool);
+  SVN_ERR(svn_io_file_create_empty(file_path, pool));
+
+  if (file_readonly)
+    SVN_ERR(svn_io_set_file_read_only(file_path, FALSE, pool));
+
+  if (dir_readonly)
+    SVN_ERR(svn_io_set_file_read_only(sub_dir_path, FALSE, pool));
+
+  *dir_path = sub_dir_path;
+  return SVN_NO_ERROR;
+}
+
+
 
 /* Functions to check the 2-way and 3-way file comparison functions.  */
 
@@ -1147,6 +1179,56 @@ test_apr_trunc_workaround(apr_pool_t *po
   return SVN_NO_ERROR;  
 }
 
+
+/* Issue #4806 */
+static svn_error_t *
+test_rmtree_all_writable(apr_pool_t *pool)
+{
+  const char *dir_path = NULL;
+
+  SVN_ERR(create_dir_tree(&dir_path, "test_rmtree_all_writable",
+                          FALSE, FALSE, pool));
+  SVN_ERR(svn_io_remove_dir2(dir_path, FALSE, NULL, NULL, pool));
+  return SVN_NO_ERROR;
+}
+
+/* Issue #4806 */
+static svn_error_t *
+test_rmtree_file_readonly(apr_pool_t *pool)
+{
+  const char *dir_path = NULL;
+
+  SVN_ERR(create_dir_tree(&dir_path, "test_rmtree_file_readonly",
+                          FALSE, TRUE, pool));
+  SVN_ERR(svn_io_remove_dir2(dir_path, FALSE, NULL, NULL, pool));
+  return SVN_NO_ERROR;
+}
+
+/* Issue #4806 */
+static svn_error_t *
+test_rmtree_dir_readonly(apr_pool_t *pool)
+{
+  const char *dir_path = NULL;
+
+  SVN_ERR(create_dir_tree(&dir_path, "test_rmtree_dir_readonly",
+                          TRUE, FALSE, pool));
+  SVN_ERR(svn_io_remove_dir2(dir_path, FALSE, NULL, NULL, pool));
+  return SVN_NO_ERROR;
+}
+
+/* Issue #4806 */
+static svn_error_t *
+test_rmtree_all_readonly(apr_pool_t *pool)
+{
+  const char *dir_path = NULL;
+
+  SVN_ERR(create_dir_tree(&dir_path, "test_rmtree_all_readonly",
+                          TRUE, TRUE, pool));
+  SVN_ERR(svn_io_remove_dir2(dir_path, FALSE, NULL, NULL, pool));
+  return SVN_NO_ERROR;
+}
+
+
 /* The test table.  */
 
 static int max_threads = 3;
@@ -1184,6 +1266,14 @@ static struct svn_test_descriptor_t test
                    "test svn_io_open_uniquely_named()"),
     SVN_TEST_PASS2(test_apr_trunc_workaround,
                    "test workaround for APR in svn_io_file_trunc"),
+    SVN_TEST_PASS2(test_rmtree_all_writable,
+                   "test svn_io_remove_dir2() with writable tree"),
+    SVN_TEST_PASS2(test_rmtree_file_readonly,
+                   "test svn_io_remove_dir2() with read-only file"),
+    SVN_TEST_PASS2(test_rmtree_dir_readonly,
+                   "test svn_io_remove_dir2() with read-only directory"),
+    SVN_TEST_PASS2(test_rmtree_all_readonly,
+                   "test svn_io_remove_dir2() with read-only tree"),
     SVN_TEST_NULL
   };
 



Mime
View raw message