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.
>
> > {
> >
>
>
diff -ur subversion-1.6.15.orig/subversion/libsvn_repos/repos.c
subversion-1.6.15/subversion/libsvn_repos/repos.c
--- subversion-1.6.15.orig/subversion/libsvn_repos/repos.c 2010-03-16
15:22:28.000000000 +0000
+++ subversion-1.6.15/subversion/libsvn_repos/repos.c 2010-12-07
18:13:30.173736691 +0000
@@ -1251,7 +1251,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)
{
diff -ur subversion-1.6.15.orig/subversion/mod_dav_svn/repos.c
subversion-1.6.15/subversion/mod_dav_svn/repos.c
--- subversion-1.6.15.orig/subversion/mod_dav_svn/repos.c 2010-11-10
14:56:31.000000000 +0000
+++ subversion-1.6.15/subversion/mod_dav_svn/repos.c 2010-12-07
18:52:09.487936650 +0000
@@ -1014,34 +1014,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_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. */
- magic_component = relative + 1;
- relative = "/";
- }
- else
- {
- magic_component = apr_pstrndup(r->pool, relative + 1,
- magic_end - relative - 1);
- relative = magic_end;
- }
+ 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;
+
+ 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_name = magic_component;
@@ -1224,6 +1252,7 @@
droot->rev = SVN_INVALID_REVNUM;
comb->priv.repos = repos;
+ repos->root_path = root_path;
repos->pool = r->pool;
repos->xslt_uri = dav_svn__get_xslt_uri(r);
repos->autoversioning = dav_svn__get_autoversioning_flag(r);
@@ -1622,7 +1651,7 @@
dav_resource_combined *comb;
dav_svn_repos *repos;
const char *cleaned_uri;
- const char *repos_name;
+ const char *repos_name = NULL;
const char *relative;
const char *repos_path;
const char *repos_key;
@@ -1638,9 +1667,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,
+ &repos_name, &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) && !repos_name)
{
char *uri = apr_pstrdup(r->pool, r->uri);
char *parentpath = apr_pstrdup(r->pool, root_path);
@@ -1653,13 +1688,10 @@
if (parentpath[parentpath_len-1] == '/')
parentpath[parentpath_len-1] = '\0';
- if (strcmp(parentpath, uri) == 0)
- {
- err = get_parentpath_resource(r, root_path, resource);
- if (err)
- return err;
- return NULL;
- }
+ err = get_parentpath_resource(r, root_path, resource);
+ if (err)
+ return err;
+ return NULL;
}
/* This does all the work of interpreting/splitting the request uri. */
@@ -2882,8 +2914,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_dirents2(&dirents, fs_parent_path, resource->pool);
if (serr != NULL)
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)
{
