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

Re: Input validation observations

From: Noorul Islam K M <noorul_at_collab.net>
Date: Sat, 04 Dec 2010 12:34:08 +0530

Julian Foad <julian.foad_at_wandisco.com> writes:

> Noorul Islam K M wrote:
>
>> Julian Foad <julian.foad_at_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?
>

Attached is the updated patch using svn_err_best_message()

Log
[[[

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.

Patch by: Noorul Islam K M <noorul{_AT_}collab.net>
]]]

Thanks and Regards
Noorul

Index: subversion/svn/info-cmd.c
===================================================================
--- subversion/svn/info-cmd.c (revision 1041944)
+++ subversion/svn/info-cmd.c (working copy)
@@ -504,6 +504,7 @@
   svn_opt_revision_t peg_revision;
   svn_info_receiver_t receiver;
   const char *path_prefix;
+ char errbuf[256];
 
   SVN_ERR(svn_cl__args_to_target_array_print_reserved(&targets, os,
                                                       opt_state->targets,
@@ -584,12 +585,9 @@
             }
           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"),
+ svn_err_best_message(err, errbuf, sizeof(errbuf))));
             }
           else
             {
Received on 2010-12-04 08:04:58 CET

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.