Issue #2154 - svn blame to error out gracefully given a directory

[VK Sameer <sa...@collab.net>]

Hello,

I have a couple of questions about this issue 
(http://subversion.tigris.org/issues/show_bug.cgi?id=2154).

While debugging this, verbosity on the client side could be increased 
with neon_debug_mask. What is the equivalent on the server side?
Alternatively, not having delved enough into the code, any pointers to 
the area to instrument to find out what goes over the wire from the 
server end? Looking at an Ethereal trace doesn't give all of the info
contained in this error message:

subversion/libsvn_ra_dav/util.c:682: (apr_err=175002)
svn: REPORT request failed on '/repositories/blame_tests-3/!svn/bc/1/A'
subversion/libsvn_ra_dav/util.c:668: (apr_err=175002)
svn: The REPORT request returned invalid XML in the response: XML parse 
error at line 1: Extra content at the end of the document
. (/repositories/blame_tests-3/!svn/bc/1/A)

The error messages show up in the httpd server logs, but don't get sent
over the wire.
dav_svn__file_revs_report() does generate the correct error message and
a status of 500. I don't understand all the details, but it looks like 
it is not possible to modify the header at that point. Can this be put
into the HTTP packet's body as an xml error message?

There also seems to be an inconsistency in the way dav_svn_convert_err() 
handles error messages:

     derr = dav_new_error_tag(pool, status,
                              serr->apr_err, apr_pstrdup(pool, 
serr->message),
                              SVN_DAV_ERROR_NAMESPACE,
                              SVN_DAV_ERROR_TAG);
     if (message != NULL)
         derr = dav_push_error(pool, status, serr->apr_err,
                               message, derr);

As currently implemented for this issue, message and serr->message are 
both NULL (and in fact message = serr->message). There is no check for 
NULL before calling apr_pstrdup(). Is this a problem?

Regards
Sameer





---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: Issue #2154 - svn blame to error out gracefully given a directory

[kf...@collab.net]

VK Sameer <sa...@collab.net> writes:
> As currently implemented for this issue, message and serr->message are
> both NULL (and in fact message = serr->message). There is no check for
> NULL before calling apr_pstrdup(). Is this a problem?


httpd-2.0/srclib/apr/include/apr_strings.h doesn't seem to say:

   /**
    * duplicate a string into memory allocated out of a pool
    * @param p The pool to allocate out of
    * @param s The string to duplicate
    * @return The new string
    */
   APR_DECLARE(char *) apr_pstrdup(apr_pool_t *p, const char *s);
   
httpd-2.0/srclib/apr/strings/apr_strings.c, however, does:

   APR_DECLARE(char *) apr_pstrdup(apr_pool_t *a, const char *s)
   {
       char *res;
       apr_size_t len;
   
       if (s == NULL) {
           return NULL;
       }
       len = strlen(s) + 1;
       res = apr_palloc(a, len);
       memcpy(res, s, len);
       return res;
   }

I still think it's a bug that we don't check serr->message in our
code, however, and rely on this undocumented behavior of apr_pstrdup.
My sense is that our code "thinks" serr->message is non-null, and that
this might get us into trouble down the road in 'derr' or some other
place.

-Karl

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org