On Fri, Jun 11, 2010 at 05:37:11PM +0200, Uwe Stuehler wrote: > Meanwhile Michael Pilato and Stefan Sperling gave me even > more input (thank you) and I was able to prepare a suite of tests > modeled after others in subversion/tests in order to check most > of the other commands which, like 'add' and 'status', only accept > paths or URLs in certain argument positions.
> a) Validating all input in the libsvn_client library and gracefully > handling any errors instead of triggering an assertion allows the > client to recover. I think everyone agreed that this is good. Yes, I think so, too. > b) Checking the arguments in the CLI commands before calling > libsvn_client functions makes it possible to abort early and not > in the middle of processing the command. I agree we should check at both layers. > But svn_path_is_url() only does a syntactic check, and ^/ (or the > expanded URL) is in fact a syntactically correct POSIX pathname. > That is why c) also appeals to me. > > c) Stefan's suggested fix through canonicalization of path arguments > produces an error message that seems more rational to me than, say, > "Path '%s' is not a working copy path, but a URL" - since even a URL > is a syntactically correct pathname (on POSIX systems). However, > this would imply that target arguments aren't parsed and interpreted > in a uniform way and could lead to confusion? That is why d) also > seems plausible. The CLI client needs to canonicalise paths it passes to the client library. This is an independent issue, really. It just happens that passing canonicalised paths triggers a code path that does not run into the assertion. But we need to fix the assertion failure either way, by rejecting invalid arguments with proper error codes. > d) With consistent expansion of ^/ to the repository URL and consistent > parsing of @peg-revisions in all target arguments, and behavioral > differences depending on the kind of target given, I could understand > a policy that treats pathnames which look like URLs as syntactically > incorrect and rejects them where only local pathnames are expected > and vice versa. If we enforce a certain syntax for certain types of arguments, we should probably provide a way to relax the checks in case someone really wants to version files or directories called http://www.example.com. We don't currently impose restrictions on names of versioned nodes, and I don't think we should do so. I'd like to move forward with this. Any objections regarding the above points? Else I will try to finish this work myself, unless Uwe finds time to continue it soon. Stefan
