Bug#359679: libsvn-core-perl: status(".", ...) gives an assert() error

[Max Bowsher]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Peter Samuelson wrote:
> [Julian Gilbey]
>> burnside:~/debian/tex/tetex-bin $ perl -MSVN::Client -e \
>>   'sub print_names { print "<$_[0]>\n"; } $ctx=new SVN::Client;
>>   $ctx->status("", "BASE", \&print_names, 1, 1, 0, 1);' | head -5
>> <>
>> <.pc>
>> <.pc/.version>
>> <configure>
>> <INSTALL.generic>
> 
> I reproduced your bugs with subversion 1.3.0-5, so I can properly
> discuss it with upstream.  Unfortunately, I don't know much about the
> Perl bindings.  But comparing "svn status" with your command, it does
> seem to correspond to the "." entry.  I wonder if that is even
> considered a bug.  I mean, if you prefix each entry with $(pwd)/, it is
> fine.
> 
> 
>> perl: /tmp/buildd/subversion-1.2.3dfsg1/subversion/libsvn_subr/path.c:377:
>> svn_path_basename: Assertion `is_canonical (path, len)' failed.
> 
> Right, that's definitely a bug.  Even if this isn't something the perl
> bindings can fix on their own, they should carp or something.

Hi. This is an issue that gets kicked around on dev at svn from time to
time, and usually ends up with the thread fizzling out sooner or later,
with no concrete action being taken, due to a lack of an obvious right
way to proceed. I'll sum up the situation...

There exist a number of path manipulation functions (svn_path_*) which
do not conform to the usual style returning an error status as the
return value, and output data via output pointer parameters. Instead
they return data directly as the return value, and are supposed to only
ever be able to experience errors that merit assert() or abort().

Subversion defines a 'canonical path' format, and most of the functions
(apart from the canonicalizer itself, obviously!) assert that the input
path looks like a canonical path.


Various groups of C programmers will conduct heated debates on whether
this is good programming practice, or an annoyance, but that is
irrelevant where the bindings are concerned, since assert()-ing out of a
scripting language interpreter is clearly bad.

There is a fairly obvious, though non-trivial, solution:
Make the bindings test all input paths (presumably using a path-specific
SWIG "in" typemap) using the same logic as is_canonical, and
canonicalize if necessary. The problem, though, is that discussions of
this nature tend to get intertwined with the parallel issue of whether
the C code is being generally unhelpful in this situation, and should be
changed too.


OK, now you know the background.

Feel free to prod dev at svn to raise awareness of this problem which has
sadly lain dormant for far too long.


Max.

Aside:
The canonical form (IIRC) is mostly common sense stuff:
* no repeated slashes
* no internal /../ sequences
* no trailing slash
BUT it has one weird rule:
* the canonical form of "." is ""




-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.2.1 (Cygwin)

iD8DBQFEKmWdfFNSmcDyxYARApfPAJ9CQqhWW1aIOjTlJlKqO6J/huTXxwCgpqNW
wrEv0yF3THxXqnqTDxY4Sh0=
=MAgH
-----END PGP SIGNATURE-----