Thanks for acting on this! I wish I had the time to actually fix the bugs people show me these days.
On Wed, Jul 21, 2010 at 2:40 PM, <rhuij...@apache.org> wrote: > Author: rhuijben > Date: Wed Jul 21 21:40:10 2010 > New Revision: 966431 > > URL: http://svn.apache.org/viewvc?rev=966431&view=rev > Log: > Following up on r867727 (aka r27653), fix some issues noted in > http://svn.haxx.se/dev/archive-2010-07/0385.shtml > > The reason I'm fixing this, is that AnkhSVN might use this function > from it's diff viewer when hitting these very old servers. > > * subversion/libsvn_ra/compat.c > (svn_ra__file_revs_from_log): Avoid NULL reference on calling this > function on the repos root, by checking if the path is a file before > the other tests. And then provide the path to svn_ra_get_log2(), to get > the changes on the requested target instead of on the session root. > > Modified: > subversion/trunk/subversion/libsvn_ra/compat.c > > Modified: subversion/trunk/subversion/libsvn_ra/compat.c > URL: > http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_ra/compat.c?rev=966431&r1=966430&r2=966431&view=diff > ============================================================================== > --- subversion/trunk/subversion/libsvn_ra/compat.c (original) > +++ subversion/trunk/subversion/libsvn_ra/compat.c Wed Jul 21 21:40:10 2010 > @@ -663,24 +663,24 @@ svn_ra__file_revs_from_log(svn_ra_sessio > svn_stream_t *last_stream; > apr_pool_t *currpool, *lastpool; > > + /* Check to make sure we're dealing with a file. */ > + SVN_ERR(svn_ra_check_path(ra_session, path, end, &kind, pool)); > + > + if (kind == svn_node_dir) > + return svn_error_createf(SVN_ERR_FS_NOT_FILE, NULL, > + _("'%s' is not a file"), repos_abs_path); What about svn_node_none? > + > SVN_ERR(svn_ra_get_repos_root2(ra_session, &repos_url, pool)); > SVN_ERR(svn_ra_get_session_url(ra_session, &session_url, pool)); > > /* Create the initial path, using the repos_url and session_url */ > - tmp = svn_path_is_child(repos_url, session_url, pool); > + tmp = svn_uri_is_child(repos_url, session_url, pool); > repos_abs_path = apr_palloc(pool, strlen(tmp) + 1); Segfaults if repos_url == session_url. > repos_abs_path[0] = '/'; > memcpy(repos_abs_path + 1, tmp, strlen(tmp)); > > - /* Check to make sure we're dealing with a file. */ > - SVN_ERR(svn_ra_check_path(ra_session, "", end, &kind, pool)); > - > - if (kind == svn_node_dir) > - return svn_error_createf(SVN_ERR_FS_NOT_FILE, NULL, > - _("'%s' is not a file"), repos_abs_path); > - > condensed_targets = apr_array_make(pool, 1, sizeof(const char *)); > - APR_ARRAY_PUSH(condensed_targets, const char *) = ""; > + APR_ARRAY_PUSH(condensed_targets, const char *) = path; > > lmb.path = svn_path_uri_decode(repos_abs_path, pool); Hmm, I would have thought you might need to stick "path" onto this as well. Have you tested this fix? I just asked the person who found this bug to give me an example of a publicly accessible ancient server that could be used for testing. > lmb.eldest = NULL; > > > -- glas...@davidglasser.net | langtonlabs.org | flickr.com/photos/glasser/
