subversion-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Julian Foad <julian.f...@wandisco.com>
Subject Re: Input validation observations
Date Fri, 03 Dec 2010 11:51:54 GMT
Noorul Islam K M wrote:
> Julian Foad <julian.foad@wandisco.com> writes:
> >   * "svn info ^/b" -> "Not a valid URL"; should be "path '/b' not found
> > in revision REV"?
> 
> Patch attached.

Hi Noorul.

> Make "svn info" to display the actual error message returned by API when
> illegal URL is passed.
> 
> * subversion/svn/info-cmd.c
>   (svn_cl__info): Display the actual error message returned by
>   svn_client_info3() instead of a custom one.
[...]
> Index: subversion/svn/info-cmd.c
> ===================================================================
> --- subversion/svn/info-cmd.c	(revision 1041293)
> +++ subversion/svn/info-cmd.c	(working copy)
> @@ -584,12 +584,8 @@
>              }
>            else if (err->apr_err == SVN_ERR_RA_ILLEGAL_URL)
>              {
> -              SVN_ERR(svn_cmdline_fprintf
> -                      (stderr, subpool,
> -                       _("%s:  (Not a valid URL)\n\n"),
> -                       svn_path_is_url(truepath)
> -                         ? truepath
> -                         : svn_dirent_local_style(truepath, pool)));
> +		    SVN_ERR(svn_cmdline_fprintf (stderr, subpool, 
> +                                                 _("%s\n\n"), err->message));

Unfortunately we cannot assume that err->message is a good user-friendly
description of the problem.  It appears that it *is* in the specific
case we're looking at:

$ svn info ^/b
URL 'https://svn.apache.org/repos/asf/b' non-existent in revision
1041775

That's lovely.  But in other cases it's not:

$ svn info hhh://aa.cc.dd/foo
traced call

See how err->message is just "traced call" in this case.  We can't even
assume it is not NULL, in fact.

I can think of two options.  We could try to use one of the
svn_error_*() functions to print out a "nice" description of the actual
returned error.  Alternatively, we could ignore 'err' and print a simple
message here, like the existing code is doing but saying something more
helpful than "Not a valid URL".  What do you think?

Does the documentation for svn_client_info3() say under what conditions
it returns the error code SVN_ERR_RA_ILLEGAL_URL?  Does that help?

- Julian



Mime
View raw message