Ping. This Submission has received no new comments.
On 09/12/2010, at 11:23 PM, David Burley wrote: > They got mangled on the way to my mail client -- I tested them again locally > before attaching them this time -- and they are now unmangled. > > Thank you, > > David > > On Thu, Dec 9, 2010 at 2:38 AM, Daniel Shahaf <d...@daniel.shahaf.name> wrote: > David Burley wrote on Wed, Dec 08, 2010 at 19:37:51 -0500: > > Patches attached again but can also be found here: > > > > http://subversion.tigris.org/issues/show_bug.cgi?id=3588 > > > > On Wed, Dec 8, 2010 at 5:24 PM, Daniel Shahaf > > <d...@daniel.shahaf.name>wrote: > > > > > David Burley wrote on Wed, Dec 08, 2010 at 17:03:06 -0500: > > > > So I have forward-ported Stephane's patch to add recursion to > > > SVNParentPath > > > > for 1.6.15 (see attached patch). > > > > > > The patch didn't get to the list. Please re-send it with *.txt extension. > > > > > > Thanks. > > > > > > Daniel > > > > > The 1.7.x patch doesn't apply to HEAD of trunk. > > > Index: subversion/mod_dav_svn/repos.c > > =================================================================== > > --- subversion/mod_dav_svn/repos.c (revision 1043620) > > +++ subversion/mod_dav_svn/repos.c (working copy) > > @@ -1235,34 +1235,63 @@ > > { > > /* SVNParentPath was used instead: assume the first component of > > 'relative' is the name of a repository. */ > > - const char *magic_component, *magic_end; > > + extern svn_boolean_t check_repos_path(const char *path, apr_pool_t > > *pool); > > + const char *magic_component = NULL, *magic_end; > > + char *repos_path; > > > > - /* A repository name is required here. > > - Remember that 'relative' always starts with a "/". */ > > - if (relative[1] == '\0') > > - { > > - /* ### are SVN_ERR_APMOD codes within the right numeric space? */ > > - return dav_svn__new_error(r->pool, HTTP_FORBIDDEN, > > - SVN_ERR_APMOD_MALFORMED_URI, > > - "The URI does not contain the name " > > - "of a repository."); > > - } > > + do > > + { > > + /* A repository name is required here. > > + Remember that 'relative' always starts with a "/". */ > > + if (relative[1] == '\0') > > + { > > + /* ### are SVN_ERR_APMOD codes within the right numeric > > space? */ > > + return dav_new_error(r->pool, HTTP_FORBIDDEN, > > + SVN_ERR_APMOD_MALFORMED_URI, > > + "The URI does not contain the name " > > + "of a repository."); > > + } > > + > > + magic_end = ap_strchr_c(relative + 1, '/'); > > + if (!magic_end) > > + { > > + /* ### Request was for parent directory with no trailing > > + slash; we probably ought to just redirect to same with > > + trailing slash appended. */ > > + if (!magic_component) > > + { > > + magic_component = relative + 1; > > + } > > + else > > + { > > + magic_component = apr_pstrcat(r->pool, magic_component, > > + relative, NULL); > > + } > > + relative = "/"; > > + } > > + else > > + { > > + if (!magic_component) > > + { > > + magic_component = apr_pstrndup(r->pool, relative + 1, > > + magic_end - relative - 1); > > + } > > + else > > + { > > + char *tmp_magic_component; > > > > - magic_end = ap_strchr_c(relative + 1, '/'); > > - if (!magic_end) > > - { > > - /* ### Request was for parent directory with no trailing > > - slash; we probably ought to just redirect to same with > > - trailing slash appended. */ > > - magic_component = relative + 1; > > - relative = "/"; > > - } > > - else > > - { > > - magic_component = apr_pstrndup(r->pool, relative + 1, > > - magic_end - relative - 1); > > - relative = magic_end; > > - } > > + tmp_magic_component = apr_pstrndup(r->pool, relative, > > + magic_end - relative); > > + magic_component = apr_pstrcat(r->pool, magic_component, > > + tmp_magic_component, NULL); > > + } > > + relative = magic_end; > > + } > > + > > + repos_path = svn_path_join(fs_parent_path, magic_component, > > + r->pool); > > + } > > + while (check_repos_path(repos_path, r->pool) != TRUE); > > > > /* return answer */ > > *repos_basename = magic_component; > > @@ -1881,7 +1910,7 @@ > > dav_resource_combined *comb; > > dav_svn_repos *repos; > > const char *cleaned_uri; > > - const char *repo_basename; > > + const char *repo_basename = NULL; > > const char *relative; > > const char *repos_path; > > const char *repos_key; > > @@ -1897,9 +1926,15 @@ > > xslt_uri = dav_svn__get_xslt_uri(r); > > fs_parent_path = dav_svn__get_fs_parent_path(r); > > > > + /* This does all the work of interpreting/splitting the request uri. */ > > + err = dav_svn_split_uri(r, r->uri, root_path, > > + &cleaned_uri, &had_slash, > > + &repo_basename, &relative, &repos_path); > > + > > + > > /* Special case: detect and build the SVNParentPath as a unique type > > of private resource, iff the SVNListParentPath directive is 'on'. */ > > - if (fs_parent_path && dav_svn__get_list_parentpath_flag(r)) > > + if (fs_parent_path && dav_svn__get_list_parentpath_flag(r) && > > !repo_basename) > > { > > char *uri = apr_pstrdup(r->pool, r->uri); > > char *parentpath = apr_pstrdup(r->pool, root_path); > > @@ -1912,13 +1947,10 @@ > > if (parentpath[parentpath_len-1] == '/') > > parentpath[parentpath_len-1] = '\0'; > > > > - if (strcmp(parentpath, uri) == 0) > > - { > > - err = get_parentpath_resource(r, resource); > > - if (err) > > - return err; > > - return NULL; > > - } > > + err = get_parentpath_resource(r, resource); > > + if (err) > > + return err; > > + return NULL; > > } > > > > /* This does all the work of interpreting/splitting the request uri. */ > > @@ -3161,8 +3193,13 @@ > > { > > apr_hash_index_t *hi; > > apr_hash_t *dirents; > > + int root_path_len = strlen(resource->info->repos->root_path); > > const char *fs_parent_path = > > - dav_svn__get_fs_parent_path(resource->info->r); > > + apr_pstrcat(resource->info->r->pool, > > + dav_svn__get_fs_parent_path(resource->info->r), > > + resource->info->r->uri > > + + ((root_path_len > 1) ? root_path_len : 0), > > + NULL); > > > > serr = svn_io_get_dirents3(&dirents, fs_parent_path, TRUE, > > resource->pool, resource->pool); > > Index: subversion/libsvn_repos/repos.c > > =================================================================== > > --- subversion/libsvn_repos/repos.c (revision 1043620) > > +++ subversion/libsvn_repos/repos.c (working copy) > > @@ -1428,7 +1428,7 @@ > > * on errors (which would be permission errors, probably) so that > > * we the user will see them after we try to open the repository > > * for real. */ > > -static svn_boolean_t > > +svn_boolean_t > > check_repos_path(const char *path, > > apr_pool_t *pool) > > As earlier reviewers told you, we do not define functions in one library > and use them in another unless they have a svn_ svn_foo__ name prefix. > > > { > > > > > <svnparentpath-recursion-1.6.15.patch.txt><svnparentpath-recursion-1.7.x.patch.txt>
