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

Re: svn commit: r966431 - /subversion/trunk/subversion/libsvn_ra/compat.c

From: David Glasser <glasser_at_davidglasser.net>
Date: Wed, 21 Jul 2010 20:29:14 -0700

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, <rhuijben_at_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;
>
>
>

-- 
glasser_at_davidglasser.net | langtonlabs.org | flickr.com/photos/glasser/
Received on 2010-07-22 05:30:12 CEST

This is an archived mail posted to the Subversion Dev mailing list.