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