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
};
