[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: Revision dates in URLs

From: C. Michael Pilato <cmpilato_at_collab.net>
Date: 2006-08-28 20:22:42 CEST

Nilton Volpato wrote:
> On 8/26/06, Nilton Volpato <nilton.volpato@gmail.com> wrote:
> [..]
>> This is now an issue, with number 2602.
>>
>> Attached is a proposed patch to fix this issue.
>>
>> It's working, although it needs some revision as I'm not certain I
>> used svn_stringbuf* correctly, and I don't know if I should somehow
>> free the old "target" variable at line 849.
>
> A better approach, by modifying svn_opt_parse_path. This patch
> substitutes the last one.
>
> Its working, although it may need some revision.

It does need some revision. Actually, it's needs some disambiguity of
revisions -- you've added a 'rev' variable to a function that has a
'rev' parameter. (Do you not have warnings enabled in your
compilations?) The patch also doesn't conform to our code formatting
standards (uses "if {\n ..." instead of "if\n{ ...").

> Index: subversion/libsvn_subr/opt.c
> ===================================================================
> --- subversion/libsvn_subr/opt.c (revision 21264)
> +++ subversion/libsvn_subr/opt.c (working copy)
> @@ -730,9 +730,24 @@
> }
> else /* looking at non-empty peg revision */
> {
> + char *rev = path + i + 1;
> + int rev_len = strlen(rev);
> + char *unescaped_rev;
> +
> + /* if rev.startswith('%7B') and rev.endswith('%7D') */
> + if ( rev[0] == '%' && rev[1] == '7' && rev[2] == 'B' &&
> + rev[rev_len-3] == '%' && rev[rev_len-2] == '7' &&
> + rev[rev_len-1] == 'D' ) {

Eek! There's no bounds-checking, so the patch can read off either end
of the string. Remember, not all input to this function is URI-escaped
URLs -- could be regular ol' system paths, which quite possibly can be
named just "%".

> + unescaped_rev = apr_pstrdup(pool, path+i+1);
> + unescaped_rev[2] = '{';
> + unescaped_rev[rev_len-3] = '}';
> + unescaped_rev[rev_len-2] = '\0';
> + rev = unescaped_rev + 2;
> + }
> +
> ret = svn_opt_parse_revision(&start_revision,
> &end_revision,
> - path + i + 1, pool);
> + rev, pool);
> }
>
> if (ret || end_revision.kind != svn_opt_revision_unspecified)

I'll work with this patch a little bit to clean it up. See if I can get
it committed Real Soon Now. Thanks for foundation, Nilton.

-- 
C. Michael Pilato <cmpilato@collab.net>
CollabNet   <>   www.collab.net   <>   Distributed Development On Demand

Received on Mon Aug 28 20:23:37 2006

This is an archived mail posted to the Subversion Dev mailing list.

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.