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

Re: Incorrectness in svn_opt__split_arg_at_peg_revision() [was: regression from 1.4.3 to 1.5.1 in handling filenames containg at (@) signs]

From: Troy Curtis Jr <troycurtisjr_at_gmail.com>
Date: Wed, 27 Aug 2008 22:52:33 -0400

>
> (gdb) n
> (gdb) n
> (gdb) n
> svn_client_args_to_target_array (targets_p=0xbfe05988, \
> os=0x9e45b38, \
> known_targets=0x0, \
> ctx=0x9e46328, \
> pool=0x9e45990) \
> at subversion/libsvn_client/cmdline.c:229
> (gdb) p true_target
> $11 = 0x9e47598 "dir/@file"
> (gdb) p peg_rev
> $12 = 0x9e475a8 "@"
>
> ### Mmmm. Yup. So, 'peg_rev' should really be "", right?
>
> Should I just make the obvious fix -- that is, set 'peg_start' to NULL
> if we're on the first iteration of the loop -- or is there something
> deeper going on here?
>
> -Karl
>

The problem there is with URLs containing '@'. i.e.

http://host/myrepo/@file

To see it you use

svn info http://host/myrepo/@file@

Which will work because the 'info' command calls svn_opt_parse_path()
on its arguments because it knows it might have a peg revision. The
problem with the 'add' subcommand is that is does not call
svn_opt_parse_path().

So if svn_opt__split_arg_at_peg_revision() returns NULL, then it will
break the URL case (because now the string 'http://host/myrepo/@file'
is passed to svn_opt_parse_path() which tries to parse the peg rev
'file')

If svn_opt__split_arg_at_peg_revision() returns '@' for the empty
string, then svn_opt_parse_path() will always work.

I think the better solution would be changing all the subcommands to
use svn_opt_parse_path(), even if they can't ever really accept peg
revisions. In fact, calling svn_opt_parse_path() in those cases might
provide much more clear error messages. For those subcommands that
cannot accept peg revisions, svn_opt_parse_path() returning anything
other than svn_opt_revision_unspecified would be an error that could
say:
"Peg revisions are not valid for the 'add' command."
Instead of the current:
"Path not found 'dir/myfile_at_234'"
when it interprets the entire string as a path.

Troy

--
"Beware of spyware. If you can, use the Firefox browser." - USA Today
Download now at http://getfirefox.com
Registered Linux User #354814 ( http://counter.li.org/)
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
For additional commands, e-mail: dev-help_at_subversion.tigris.org
Received on 2008-08-28 04:52:46 CEST

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.