Re: svn delete fails with "403 Forbidden" if root is not readable

I filed this as issue #4219, 'svn delete fails with "403 Forbidden" if root is not readable'. With Philip's help we determined this bug only occurs with RA-neon.  I can now reproduce it on 1.7.x. I looked into why Dmitry's patch doesn't work.  It calls svn_uri_condense_targets(), which finds the longest common URL of the targets, and reparents the RA session there.  That's not quite right: we need to parent the session at the longest common URL of the targets' *parent* directories.  (Also that function has a stupid irregularity when number of targets == 1: it returns an empty array of relpaths, so we need to work around that.) But that's still not right.  Who says we require read-write access on the longest common URL of all the parents?  I think we only want to require read-write access on the parent of each target individually.  Otherwise users would find that a multi-URL delete can fail in a case where multiple single-URL deletes succeed, and that would be needlessly restrictive and incompatible with 1.6. I went looking at the Book and found nothing about what access is required for a delete.  That's an omission we should remedy. What is the correct authorization requirement for deleting a node?  Read-write required on the node's parent directory and on the node itself?  Only on its parent?  Only on the node itself?  Read access on the parent and read-write on the node? On trunk, we no longer have Neon, so the bug does not manifest.  But is the bug really still there?  It seems to me that the implied semantics of RA sessions and authzrequire a fix here too, because it reparents the session to the repository root URL ... but I'm really not sure.  libsvn_ra and specifically svn_ra_open4() fails to document how the session root URL interacts with authorization.  What should the rule be? So, TODO:   * Decide among ourselves what the authz requirement for delete should be.   * Update the Book to document that.   * Update <svn_ra.h> to document authorization w.r.t. the session root URL.   * Adjust my test to have read-write access only where it should be required, and nowhere else.   * Write fixes for trunk and 1.7.x.   * Check for similar issues with other operations: firstly, 'move' because that's so similar to 'delete'. - Julian   To: Dmitry Pavlenko <pavlenko@tmatesoft.com> >Cc: "dev@subversion.apache.org" <dev@subversion.apache.org> >Sent: Tuesday, 14 August 2012, 22:22 >Subject: Re: svn delete fails with "403 Forbidden" if root is not readable > >Dmitry Pavlenko wrote: > >> I'd like to ping my old report. I'll recollect: if there's no read >> permission of the repository >> root, "svn delete" for sub-URL always fails. > >Hi Dmitry. > >Thanks for the report. > >I tried to reproduce this, and I cannot.  The tests I wrote are in the attached patch. > >I tested with RA-serf (with FSFS and with BDB), on an Ubuntu Linux system.  All the tests pass. > >Can you try this test, or do you have any idea what I need to do to reproduce the problem? > >- Julian > > >> I had the same issue in SVNKit but fixed, and here's the fix: >> >> svn log --diff -r9286 http://svn.svnkit.com/repos/svnkit/trunk/ >> >> I also tried to apply the same changes to Subversion,but >> it's not very comfortable for me to debug because of compilation problems (I >> have to work with SVN >> sources under chroot). >> >> There's a patch with the changes I tried to perform. Important: it DOES NOT >> work (FAILS with >> segmentation fault --- maybe I've confused something with pools, sorry I >> didn't debug) >> but shows where and how to fix the problem. The main idea: >> >> if we have several targets to delete: a/b/c/d, a/b/e/f, a/b/c/h, it's better >> to extract common >> parent "a/b" and set ra_session URL to    >> original_URL.append("a/b")    and walk using editor >> through "c/d", "e/f", "c/h" and so on. (Though >> this maybe won't work for single file URL...) >> >> Anyway, I believe my report was useful, at least a bit! >> >> [[[ >> DO NOT apply this patch directly, it doesn't work! >> ]]] >> [[[ >> Index: subversion/libsvn_client/delete.c >> =================================================================== >> --- subversion/libsvn_client/delete.c  (revision 1360990) >> +++ subversion/libsvn_client/delete.c  (working copy) >> @@ -233,6 +233,7 @@ delete_urls_multi_repos(const apr_array_header_t * >>   apr_hash_t *relpaths = apr_hash_make(pool); >>   apr_hash_index_t *hi; >>   int i; >> +  apr_pool_t* iterpool; >> >>   /* Create a hash of repos_root -> ra_session maps and repos_root -> >> relpaths >>       maps, used to group the various targets. */ >> @@ -303,6 +304,8 @@ delete_urls_multi_repos(const apr_array_header_t * >> >>       Now we iterate over the collection of sessions and do a commit for each >>       one with the collected relpaths. */ >> + >> +  iterpool = svn_pool_create(pool); >>   for (hi = apr_hash_first(pool, sessions); hi; hi = apr_hash_next(hi)) >>     { >>       const char *repos_root = svn__apr_hash_index_key(hi); >> @@ -310,10 +313,21 @@ delete_urls_multi_repos(const apr_array_header_t * >>       const apr_array_header_t *relpaths_list = >>         apr_hash_get(relpaths, repos_root, APR_HASH_KEY_STRING); >> >> -      SVN_ERR(single_repos_delete(ra_session, repos_root, relpaths_list, >> +      const char* common_path; >> +      apr_array_header_t* condensed_targets; >> +      const char* url = repos_root; >> + >> +      svn_pool_clear(iterpool); >> +      SVN_ERR(svn_uri_condense_targets(&common_path, >> &condensed_targets, relpaths_list, FALSE, >> iterpool, iterpool)); >> +      url = svn_path_url_add_component2(repos_root, common_path, pool); >> + >> +      SVN_ERR(svn_ra_reparent(ra_session, url, pool)); >> + >> +      SVN_ERR(single_repos_delete(ra_session, url, condensed_targets, >>                                   revprop_table, commit_callback, >>                                   commit_baton, ctx, pool)); >>     } >> +  svn_pool_destroy(iterpool); >> >>   return SVN_NO_ERROR; >> } >> ]]] >> >> -- >> Dmitry Pavlenko, >> TMate Software, >> http://subgit.com/ - git+svn on the server side >> >> >> В сообщении от 18 June 2012 19:20:19 автор Dmitry Pavlenko написал: >>>  Hello again, >>> >>>  I've fixed the issue in SVNKit: >> http://svn.svnkit.com/repos/svnkit/trunk/ >>>  at r9286 The approach is to use common ancestor instead of repository root >>>  to run editor calls. For example, to remove paths: >>> >>>  directory/subdirectory1/file1 >>>  directory/subdirectory1/file2 >>>  directory/subdirectory2/file3 >>> >>>  instead of running >>> >>>  openRoot(); >>>  opedDir("directory") >>> >>>  opedDir("directory/subdirectory1") >>>  delete("directory/subdirectory1/file1") >>>  delete("directory/subdirectory1/file2") >>>  closeDir() >>> >>>  opedDir("directory/subdirectory2") >>>  delete("directory/subdirectory1/file3") >>>  closeDir() >>> >>>  closeDir() >>>  closeDir() >>> >>>  on http://host/path as on the root one can run >>> >>>  opedRoot() >>> >>>  opedDir("subdirectory1") >>>  delete("subdirectory1/file1") >>>  delete("subdirectory1/file2") >>>  closeDir() >>> >>>  opedDir("subdirectory2") >>>  delete("subdirectory1/file3") >>>  closeDir() >>> >>>  closeDir() >>> >>>  http://host/path/directory as on the common ancestor. >>> >>>  > Hello, >>>  > Suppose you have a repository with authz: >>>  > >>>  > [/] >>>  > * = >>>  > [/directory] >>>  > * = rw >>>  > >>>  > And the repository (http://localhost:43714/repos) contains a directory >>>  > (with "rw" access) and a file in it. File deletion fails >> with the >>>  > following stacktrace (tried with SVN r1350986): >>>  > >>>  > $ svn rm http://localhost:43714/repos/directory/file --username user >>>  > --password password -m "" subversion/svn/delete-cmd.c:90: >>>  > (apr_err=130003) subversion/svn/util.c:913: (apr_err=130003) >>>  > subversion/libsvn_client/delete.c:409: (apr_err=130003) >>>  > subversion/libsvn_client/delete.c:315: (apr_err=130003) >>>  > subversion/libsvn_client/delete.c:217: (apr_err=130003) >>>  > subversion/libsvn_delta/deprecated.c:52: (apr_err=130003) >>>  > subversion/libsvn_delta/path_driver.c:169: (apr_err=130003) >>>  > subversion/libsvn_ra_serf/commit.c:1300: (apr_err=130003) >>>  > subversion/libsvn_ra_serf/options.c:381: (apr_err=130003) >>>  > subversion/libsvn_ra_serf/util.c:780: (apr_err=130003) >>>  > subversion/libsvn_ra_serf/util.c:737: (apr_err=130003) >>>  > subversion/libsvn_ra_serf/util.c:1980: (apr_err=130003) >>>  > subversion/libsvn_ra_serf/util.c:1961: (apr_err=130003) >>>  > subversion/libsvn_ra_serf/util.c:2418: (apr_err=130003) >>>  > svn: E130003: The OPTIONS response contains invalid XML (403 >> Forbidden) >>>  > >>>  > With SVN 1.6 everything is ok. >>>  > The problem is easily reproducible. > >
Received on 2012-08-16 17:33:06 CEST