You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by sv...@apache.org on 2019/09/20 04:00:49 UTC
svn commit: r1867197 - in /subversion/branches/1.11.x: ./ STATUS
subversion/libsvn_subr/io.c subversion/tests/libsvn_subr/io-test.c
Author: svn-role
Date: Fri Sep 20 04:00:49 2019
New Revision: 1867197
URL: http://svn.apache.org/viewvc?rev=1867197&view=rev
Log:
Merge the r1854072 group from trunk:
* r1854072, r1854074, r1854216
Fix issue #4806: Remove on-disk trees with read-only directories in them.
Justification:
Fixes an edge case in our tree removal code. If we clear read-only
permissions on files in order to remove them, we should do the same for
directories.
Votes:
+1: brane, stsp
Modified:
subversion/branches/1.11.x/ (props changed)
subversion/branches/1.11.x/STATUS
subversion/branches/1.11.x/subversion/libsvn_subr/io.c
subversion/branches/1.11.x/subversion/tests/libsvn_subr/io-test.c
Propchange: subversion/branches/1.11.x/
------------------------------------------------------------------------------
--- svn:mergeinfo (original)
+++ svn:mergeinfo Fri Sep 20 04:00:49 2019
@@ -100,4 +100,4 @@
/subversion/branches/verify-at-commit:1462039-1462408
/subversion/branches/verify-keep-going:1439280-1546110
/subversion/branches/wc-collate-path:1402685-1480384
-/subversion/trunk:1840990-1840991,1840995,1840997,1841059,1841079,1841091,1841098,1841136,1841180,1841272,1841481,1841524-1841525,1841567,1841600-1841602,1841606,1841719,1841725,1841731,1841736,1841742-1841743,1841753-1841754,1841822,1841850,1841867,1842090,1842222-1842223,1842334,1842814,1842827,1842829,1842877,1843888,1844882,1844987,1845204,1845212,1845261,1845408,1845555-1845556,1845559,1845577,1846299,1846403,1846406,1846704,1847181-1847182,1847188,1847264,1847377,1847572,1847596,1847598,1847697,1847922,1847924,1847946,1850348,1850621,1850651,1851676,1851687,1851791,1852013,1853483,1853761
+/subversion/trunk:1840990-1840991,1840995,1840997,1841059,1841079,1841091,1841098,1841136,1841180,1841272,1841481,1841524-1841525,1841567,1841600-1841602,1841606,1841719,1841725,1841731,1841736,1841742-1841743,1841753-1841754,1841822,1841850,1841867,1842090,1842222-1842223,1842334,1842814,1842827,1842829,1842877,1843888,1844882,1844987,1845204,1845212,1845261,1845408,1845555-1845556,1845559,1845577,1846299,1846403,1846406,1846704,1847181-1847182,1847188,1847264,1847377,1847572,1847596,1847598,1847697,1847922,1847924,1847946,1850348,1850621,1850651,1851676,1851687,1851791,1852013,1853483,1853761,1854072,1854074,1854216
Modified: subversion/branches/1.11.x/STATUS
URL: http://svn.apache.org/viewvc/subversion/branches/1.11.x/STATUS?rev=1867197&r1=1867196&r2=1867197&view=diff
==============================================================================
--- subversion/branches/1.11.x/STATUS (original)
+++ subversion/branches/1.11.x/STATUS Fri Sep 20 04:00:49 2019
@@ -69,15 +69,6 @@ Veto-blocked changes:
Approved changes:
=================
- * r1854072, r1854074, r1854216
- Fix issue #4806: Remove on-disk trees with read-only directories in them.
- Justification:
- Fixes an edge case in our tree removal code. If we clear read-only
- permissions on files in order to remove them, we should do the same for
- directories.
- Votes:
- +1: brane, stsp
-
* r1857367
Fix memory lifetime problem in a libsvn_wc error code path.
Justification:
Modified: subversion/branches/1.11.x/subversion/libsvn_subr/io.c
URL: http://svn.apache.org/viewvc/subversion/branches/1.11.x/subversion/libsvn_subr/io.c?rev=1867197&r1=1867196&r2=1867197&view=diff
==============================================================================
--- subversion/branches/1.11.x/subversion/libsvn_subr/io.c (original)
+++ subversion/branches/1.11.x/subversion/libsvn_subr/io.c Fri Sep 20 04:00:49 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,
+ apr_pool_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
}
@@ -2752,6 +2824,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);
@@ -4490,8 +4568,17 @@ svn_io_dir_remove_nonrecursive(const cha
{
svn_boolean_t retry = TRUE;
+ if (APR_STATUS_IS_EACCES(status) || APR_STATUS_IS_EEXIST(status))
+ {
+ /* Make the destination directory writable because Windows
+ forbids deleting read-only items. */
+ SVN_ERR(io_set_readonly_flag(dirname_apr, dirname,
+ FALSE, FALSE, TRUE, pool));
+ status = apr_dir_remove(dirname_apr, pool);
+ }
+
if (status == APR_FROM_OS_ERROR(ERROR_DIR_NOT_EMPTY))
- {
+ {
apr_status_t empty_status = dir_is_empty(dirname_apr, pool);
if (APR_STATUS_IS_ENOTEMPTY(empty_status))
Modified: subversion/branches/1.11.x/subversion/tests/libsvn_subr/io-test.c
URL: http://svn.apache.org/viewvc/subversion/branches/1.11.x/subversion/tests/libsvn_subr/io-test.c?rev=1867197&r1=1867196&r2=1867197&view=diff
==============================================================================
--- subversion/branches/1.11.x/subversion/tests/libsvn_subr/io-test.c (original)
+++ subversion/branches/1.11.x/subversion/tests/libsvn_subr/io-test.c Fri Sep 20 04:00:49 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
};