You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Garrett Rooney <ro...@electricjellyfish.net> on 2001/09/20 02:39:45 UTC
[PATCH] svn_path_canonicalize updated
here's an updated version of the canonicalization patch... this goes
back to a single canonicalize function, rather than separate ones for
urls and paths. it also fixes the one confirmed problem i found in
the path handling in the rest of the code. note, i haven't tested
with a server yet because i don't have one set up, and i haven't
tested on windows because i don't have access to a windows machine.
someone definately needs to test that before anyone even thinks about
commiting this. i also haven't checked import, since it is currently
segfaulting, but all the rest of the tests pass fine.
* util.c (svn_cl__args_to_target_array): don't canonicalize the paths
because we cannot be sure what style they are in.
* svn_path.h: update comment for svn_path_canonicalize.
* path.c (svn_path_canonicalize): actually attemt to remove redundant
portions of the path. for url and repos styles allow for the
possibility of a http:// or a file:// at the beginning of the
path.
--
garrett rooney Unix was not designed to stop you from
rooneg@electricjellyfish.net doing stupid things, because that would
http://electricjellyfish.net/ stop you from doing clever things.
Re: [PATCH] svn_path_canonicalize updated
Posted by Garrett Rooney <ro...@electricjellyfish.net>.
> > A relative URL with "//" present will throw this off.
>
> how should i tell the difference between an absolute url and a
> relative url? i suppose i could do a direct compare against http://
> https:// and file://, but that seems ugly...
i still have to think about this more, but how about advancing up to
the : (if there is one), and then skipping the // (if there is one),
then treat the rest normally?
(completely untested...)
char *tmp;
tmp = strstr(path->data, ":");
if (tmp = NULL)
tmp = path->data;
if (*tmp == '/' && *(tmp + 1) == '/')
tmp += 2;
/* now continue normally... */
i'll have to think more about this after work...
--
garrett rooney Unix was not designed to stop you from
rooneg@electricjellyfish.net doing stupid things, because that would
http://electricjellyfish.net/ stop you from doing clever things.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] svn_path_canonicalize updated
Posted by Daniel Stenberg <da...@haxx.se>.
On Thu, 20 Sep 2001, Garrett Rooney wrote:
> > > + if (style == svn_path_url_style || style == svn_path_repos_style) {
> > > + str = strstr (path->data, "//");
> >
> > A relative URL with "//" present will throw this off.
>
> how should i tell the difference between an absolute url and a relative
> url? i suppose i could do a direct compare against http:// https:// and
> file://, but that seems ugly...
You could probably check if the string begins with "scheme://", where scheme
may consist of ( alpha | digit | "+" | "-" | "." ) (according to RFC2396
section 3.1)
At least that's more generic.
--
Daniel Stenberg - http://daniel.haxx.se - +46-705-44 31 77
ech`echo xiun|tr nu oc|sed 'sx\([sx]\)\([xoi]\)xo un\2\1 is xg'`ol
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] svn_path_canonicalize updated
Posted by Garrett Rooney <ro...@electricjellyfish.net>.
On Wed, Sep 19, 2001 at 11:13:47PM -0700, Greg Stein wrote:
> On Wed, Sep 19, 2001 at 10:39:45PM -0400, Garrett Rooney wrote:
> >...
> > --- subversion/clients/cmdline/SVN/text-base/util.c Mon Sep 10 23:07:52 2001
> > +++ subversion/clients/cmdline/util.c Wed Sep 19 21:05:13 2001
> > @@ -158,7 +158,6 @@
> > svn_stringbuf_t *target = svn_stringbuf_create (os->argv[os->ind], pool);
> > svn_stringbuf_t *basename;
> >
> > - svn_path_canonicalize (target, svn_path_local_style);
> > basename = svn_path_last_component (target, svn_path_local_style, pool);
>
> There are probably some repercussions from doing this. Note that the next
> call is to svn_path_last_component(). That function may fail if passed a
> non-canonical path.
>
> Also note that it uses svn_path_local_style, too. IOW, it will be just as
> broken as the call to the canonicalize. Somebody needs to get in there and
> do some real review and fixing.
ack, you're right... this is ugly... i'll have to take a closer
look...
> >...
> > --- subversion/libsvn_subr/SVN/text-base/path.c Tue Sep 18 17:38:56 2001
> > +++ subversion/libsvn_subr/path.c Wed Sep 19 20:55:17 2001
> > @@ -74,11 +74,28 @@
> > {
> > char dirsep = get_separator_from_style (style);
> >
> > - /* At some point this could eliminiate redundant components.
> > - For now, it just makes sure there is no trailing slash. */
> > + char sep_sep[3] = { dirsep, dirsep, '\0' };
> > + char sep_dot_sep[0] = { dirsep, '.', dirsep, '\0' };
> >
> > - /* kff todo: maybe should be implemented with a new routine in
> > - libsvn_string. */
> > + char *str = NULL, *tmp = NULL;
> > +
> > + if (style == svn_path_url_style || style == svn_path_repos_style) {
> > + str = strstr (path->data, "//");
>
> A relative URL with "//" present will throw this off.
how should i tell the difference between an absolute url and a
relative url? i suppose i could do a direct compare against http://
https:// and file://, but that seems ugly...
> >...
> > + while ((tmp = strstr (str, sep_sep)) != NULL)
> > + strcpy (tmp, tmp + 1);
> > +
> > + while ((tmp = strstr (str, sep_dot_sep)) != NULL)
> > + strcpy (tmp, tmp + 2);
>
> strcpy() is not guaranteed to work properly when the two strings overlap.
good point, i'll change that to use memmove, which is guaranteed...
thanks,
-garrett
--
garrett rooney Unix was not designed to stop you from
rooneg@electricjellyfish.net doing stupid things, because that would
http://electricjellyfish.net/ stop you from doing clever things.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] svn_path_canonicalize updated
Posted by Greg Stein <gs...@lyra.org>.
On Wed, Sep 19, 2001 at 10:39:45PM -0400, Garrett Rooney wrote:
>...
> --- subversion/clients/cmdline/SVN/text-base/util.c Mon Sep 10 23:07:52 2001
> +++ subversion/clients/cmdline/util.c Wed Sep 19 21:05:13 2001
> @@ -158,7 +158,6 @@
> svn_stringbuf_t *target = svn_stringbuf_create (os->argv[os->ind], pool);
> svn_stringbuf_t *basename;
>
> - svn_path_canonicalize (target, svn_path_local_style);
> basename = svn_path_last_component (target, svn_path_local_style, pool);
There are probably some repercussions from doing this. Note that the next
call is to svn_path_last_component(). That function may fail if passed a
non-canonical path.
Also note that it uses svn_path_local_style, too. IOW, it will be just as
broken as the call to the canonicalize. Somebody needs to get in there and
do some real review and fixing.
>...
> --- subversion/libsvn_subr/SVN/text-base/path.c Tue Sep 18 17:38:56 2001
> +++ subversion/libsvn_subr/path.c Wed Sep 19 20:55:17 2001
> @@ -74,11 +74,28 @@
> {
> char dirsep = get_separator_from_style (style);
>
> - /* At some point this could eliminiate redundant components.
> - For now, it just makes sure there is no trailing slash. */
> + char sep_sep[3] = { dirsep, dirsep, '\0' };
> + char sep_dot_sep[0] = { dirsep, '.', dirsep, '\0' };
>
> - /* kff todo: maybe should be implemented with a new routine in
> - libsvn_string. */
> + char *str = NULL, *tmp = NULL;
> +
> + if (style == svn_path_url_style || style == svn_path_repos_style) {
> + str = strstr (path->data, "//");
A relative URL with "//" present will throw this off.
>...
> + while ((tmp = strstr (str, sep_sep)) != NULL)
> + strcpy (tmp, tmp + 1);
> +
> + while ((tmp = strstr (str, sep_dot_sep)) != NULL)
> + strcpy (tmp, tmp + 2);
strcpy() is not guaranteed to work properly when the two strings overlap.
Cheers,
-g
--
Greg Stein, http://www.lyra.org/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org