You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Peter Allin <pe...@arbejdskamp.dk> on 2002/12/31 09:51:33 UTC
Working on issue #946
Hi,
I've been browsing the Subversion source code for the last couple of
days. To get a place to start I've worked a bit on issue #946.
As mentioned on Issuezilla I've found out that the 405 error is
returned, when I try to checkout (or otherwise access) an URL that
doesn't point at a repository.
It seems the error code is generated in the following code from
libsvn_ra_dav/props.c in svn_ra_dav__get_baseline_props() (starting
from line 514 in revision 4213):
{
/* Try to get the starting_props from the public url. If the
resource no longer exists in HEAD, we'll get a failure. That's
fine: just keep removing components and trying to get the
starting_props from parent directories. */
svn_error_t *err;
apr_size_t len;
svn_stringbuf_t *path_s = svn_stringbuf_create (parsed_url.path,
pool);
while (! svn_path_is_empty (path_s->data))
{
err = svn_ra_dav__get_starting_props(&rsrc, sess, path_s->data,
NULL, pool);
if (! err)
break; /* found an existing parent! */
if (err->apr_err != SVN_ERR_RA_DAV_REQUEST_FAILED)
return err; /* found a _real_ error */
/* else... lop off the basename and try again. */
lopped_path = svn_path_join(svn_path_basename (path_s->data,
pool),
lopped_path,
pool);
len = path_s->len;
svn_path_remove_component(path_s);
if (path_s->len == len)
/* whoa, infinite loop, get out. */
return err;
svn_error_clear (err);
}
if (svn_path_is_empty (path_s->data))
{
/* entire URL was bogus; not a single part of it exists in
the repository! */
err = svn_error_createf(SVN_ERR_RA_ILLEGAL_URL, NULL,
"No part of path '%s' was found in "
"repository HEAD.", parsed_url.path);
ne_uri_free(&parsed_url);
return err;
}
ne_uri_free(&parsed_url);
}
Where the call to svn_ra_dav__get_starting_props() returns a 405
error. The the while-loop starts removing components from the path, but
each of the thus generated paths also results in a 405. The while loop
continues until the path is cut down to "/". Then the
svn_path_remove_component() function returns "/" and the "whoa,
infinite loop, get out" return is executed with the 405 errorcode.
The svn_path_is_empty() function never returns TRUE in this
scenario because the path is only cut down to "/" not "". Is the
condition in the while statement meant to check for this scenario or
something else? If I understand the comment at the top and the
returned error message in the bottom correctly, it's meant to check
for some other condition.
Would it be OK to return an error saying something like "The URL %s is
not part of any repository" if the path becomes "/" without the
svn_ra_dav__get_starting_props() succeeding? As far as I can see it
would work - but maybe it breaks some outher usage of the
svn_ra_dav__get_baseline_props() function?
/Peter Allin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Working on issue #946
Posted by Greg Stein <gs...@lyra.org>.
On Sat, Jan 04, 2003 at 08:38:27PM -0800, Bill Tutt wrote:
> > From: Peter Allin [mailto:peter@arbejdskamp.dk]
>...
> > Yes - I agree that the return code from the server shouldn't
> > be changed, but merely the presentation of the error shown to
> > the user.
> >
> > Should the user actually see any of the HTTP error codes, or
> > should all errors be "translated" into something more descriptive?
>
> The user should always see the HTTP error codes. The reason behind this
> is that proxy errors need to be communicated somehow, and HTTP reason
> phrases often contain this information. Useful error messages shouldn't
> come from the ra_dav client code. Useful error messages should be part
> of the HTTP reason phrase or as part of the XML response content.
Right. The client should *add* information to the error messages rather than
block them out. That is what the "wrap" is all about -- adding more info.
Cheers,
-g
--
Greg Stein, http://www.lyra.org/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Working on issue #946
Posted by Michael Wood <mw...@its.uct.ac.za>.
On Sun, Jan 05, 2003 at 01:40:03AM +0100, Peter Allin wrote:
> On Fri, 2003-01-03 at 18:01, Ben Collins-Sussman wrote:
>
> > And as Greg Stein mentioned in the issue, this is correct. 405 means
> > that there's no dav resource at that location (i.e. no repository).
> > 404 means you're looking in a dav-y area, but the resource dosen't
> > exist.
> > Still, I guess it would be nice to throw *descriptive* error strings
> > to the user, which is what you're trying to do. :-)
>
> Yes - I agree that the return code from the server shouldn't be changed,
> but merely the presentation of the error shown to the user.
>
> Should the user actually see any of the HTTP error codes, or should all
> errors be "translated" into something more descriptive?
I'd say the HTTP error codes should be shown in addition to the
descriptive messages, since 405 could mean it's not a repository, or it
could mean your proxy server is not set up properly, or it could mean
you have misconfigured Apache. It's annoying having to interpret the
descriptive message. (Have you ever tried to figure out what Internet
Explorer means by "Page cannot be displayed?")
--
Michael Wood <mw...@its.uct.ac.za>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
RE: Working on issue #946
Posted by Bill Tutt <ra...@lyra.org>.
> From: Peter Allin [mailto:peter@arbejdskamp.dk]
>
> On Fri, 2003-01-03 at 18:01, Ben Collins-Sussman wrote:
>
> > And as Greg Stein mentioned in the issue, this is correct.
> 405 means
> > that there's no dav resource at that location (i.e. no repository).
> > 404 means you're looking in a dav-y area, but the resource dosen't
> > exist. Still, I guess it would be nice to throw *descriptive* error
> > strings to the user, which is what you're trying to do. :-)
>
> Yes - I agree that the return code from the server shouldn't
> be changed, but merely the presentation of the error shown to
> the user.
>
> Should the user actually see any of the HTTP error codes, or
> should all errors be "translated" into something more descriptive?
>
The user should always see the HTTP error codes. The reason behind this
is that proxy errors need to be communicated somehow, and HTTP reason
phrases often contain this information. Useful error messages shouldn't
come from the ra_dav client code. Useful error messages should be part
of the HTTP reason phrase or as part of the XML response content.
Bill
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Working on issue #946
Posted by Peter Allin <pe...@arbejdskamp.dk>.
On Fri, 2003-01-03 at 18:01, Ben Collins-Sussman wrote:
> And as Greg Stein mentioned in the issue, this is correct. 405 means
> that there's no dav resource at that location (i.e. no repository).
> 404 means you're looking in a dav-y area, but the resource dosen't
> exist.
> Still, I guess it would be nice to throw *descriptive* error strings
> to the user, which is what you're trying to do. :-)
Yes - I agree that the return code from the server shouldn't be changed,
but merely the presentation of the error shown to the user.
Should the user actually see any of the HTTP error codes, or should all
errors be "translated" into something more descriptive?
> anything. Why not just wrap the error in the infinite-loop-break-out
> routine?
>
> > svn_path_remove_component(path_s);
> > if (path_s->len == len)
> > /* whoa, infinite loop, get out. */
> > return err;
> instead of 'return err', how about
> return svn_error_quick_wrap (err, "The path was not part of repository")
I tried this earlier today. It seems to work as expected. The output
when trying to do a checkout from an URL that isn't a DAV resource is
(there's no repository at /svn):
% svn co http://localhost:8000/svn/nothere
subversion/libsvn_ra_dav/props.c:540: (apr_err=175002)
svn: RA layer request failed
svn: The path was not part of repository
subversion/libsvn_ra_dav/util.c:81: (apr_err=175002)
svn: PROPFIND of /: 405 Method Not Allowed
I also tried this in place of "return err;"
return svn_error_createf(SVN_ERR_RA_ILLEGAL_URL, NULL,
"The path %s is not part of a repository",
parsed_url.path);
This results in the following output:
% svn co http://localhost:8000/svn/nothere
subversion/libsvn_ra_dav/props.c:539: (apr_err=170000)
svn: Bad URL passed to RA layer
svn: The path /svn/nothere is not part of a repository
Personally I think the second output is more descriptive. Both seem to
work fine and passes a "make check".
Should I make a small patch out of one of these changes?
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Working on issue #946
Posted by Ben Collins-Sussman <su...@collab.net>.
Peter Allin <pe...@arbejdskamp.dk> writes:
> As mentioned on Issuezilla I've found out that the 405 error is
> returned, when I try to checkout (or otherwise access) an URL that
> doesn't point at a repository.
And as Greg Stein mentioned in the issue, this is correct. 405 means
that there's no dav resource at that location (i.e. no repository).
404 means you're looking in a dav-y area, but the resource dosen't
exist.
Still, I guess it would be nice to throw *descriptive* error strings
to the user, which is what you're trying to do. :-)
> Where the call to svn_ra_dav__get_starting_props() returns a 405
> error. The the while-loop starts removing components from the path, but
> each of the thus generated paths also results in a 405. The while loop
> continues until the path is cut down to "/". Then the
> svn_path_remove_component() function returns "/" and the "whoa,
> infinite loop, get out" return is executed with the 405 errorcode.
Correct.
> Would it be OK to return an error saying something like "The URL %s is
> not part of any repository" if the path becomes "/" without the
> svn_ra_dav__get_starting_props() succeeding? As far as I can see it
> would work - but maybe it breaks some outher usage of the
> svn_ra_dav__get_baseline_props() function?
That sounds like a good strategy to me. I don't think it will break
anything. Why not just wrap the error in the infinite-loop-break-out
routine?
> svn_path_remove_component(path_s);
> if (path_s->len == len)
> /* whoa, infinite loop, get out. */
> return err;
instead of 'return err', how about
return svn_error_quick_wrap (err, "The path was not part of repository")
or something?
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org