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