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
Received on 2010-07-13 17:02:55 CEST