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

Re: [PATCH] fix for interactive merge-callback not supporting spaces in file paths

From: Ben Collins-Sussman <sussman_at_red-bean.com>
Date: 2007-08-06 16:50:27 CEST

Applied in r25960.

On 7/25/07, Augie Fackler <durin42@gmail.com> wrote:
> Here's a new version with the documentation as requested.
>
> [[[
> Fix issue #2816: Interactive Merge Callback Handles Paths Safely
>
> * subversion/svn/util.c
> (svn_cl__edit_file_externally) Change to the directory containing the
> temporary file and edit from there, then change back to the user's
> working
> directory.
> ]]]
>
>
> Index: subversion/svn/util.c
> ===================================================================
> --- subversion/svn/util.c (revision 25835)
> +++ subversion/svn/util.c (working copy)
> @@ -142,21 +142,49 @@
> }
>
>
> -
> +/* Use the visual editor to edit files. This requires that the file
> name itself
> + be shell-safe, although the path to reach that file need not be
> shell-safe.
> + */
> svn_error_t *
> svn_cl__edit_file_externally(const char *path,
> const char *editor_cmd,
> apr_hash_t *config,
> apr_pool_t *pool)
> {
> - const char *editor, *cmd;
> + const char *editor, *cmd, *base_dir, *file_name, *base_dir_apr;
> + char *old_cwd;
> int sys_err;
> + apr_status_t apr_err;
> +
> + svn_path_split(path, &base_dir, &file_name, pool);
>
> SVN_ERR(find_editor_binary(&editor, editor_cmd, config));
>
> - cmd = apr_psprintf(pool, "%s %s", editor, path);
> + apr_err = apr_filepath_get(&old_cwd, APR_FILEPATH_NATIVE, pool);
> + if (apr_err)
> + return svn_error_wrap_apr(apr_err, _("Can't get working
> directory"));
> +
> + /* APR doesn't like "" directories */
> + if (base_dir[0] == '\0')
> + base_dir_apr = ".";
> + else
> + SVN_ERR(svn_path_cstring_from_utf8(&base_dir_apr, base_dir, pool));
> +
> + apr_err = apr_filepath_set(base_dir_apr, pool);
> + if (apr_err)
> + return svn_error_wrap_apr
> + (apr_err, _("Can't change working directory to '%s'"), base_dir);
> +
> + cmd = apr_psprintf(pool, "%s \'%s\'", editor, file_name);
> sys_err = system(cmd);
> - if (sys_err != 0)
> +
> + apr_err = apr_filepath_set(old_cwd, pool);
> + if (apr_err)
> + svn_handle_error2(svn_error_wrap_apr
> + (apr_err, _("Can't restore working directory")),
> + stderr, TRUE /* fatal */, "svn: ");
> +
> + if (sys_err)
> /* Extracting any meaning from sys_err is platform specific, so
> just
> use the raw value. */
> return svn_error_createf(SVN_ERR_EXTERNAL_PROGRAM, NULL,
>
>
>
>
> On Jul 25, 2007, at 11:18 AM, Augie Fackler wrote:
>
> >
> > On Jul 25, 2007, at 11:16 AM, David Glasser wrote:
> >
> >> On 7/25/07, Augie Fackler <durin42@gmail.com> wrote:
> >>> The single quotes aren't actually needed for this patch, that's an
> >>> artifact from way back when I isolated the bug. New version
> >>> included.
> >>> The file name we'll be editing in this callback is one of our
> >>> tempfiles, so we know it'll have a safe name.
> >>
> >> Oh, good point. Do you think that the documentation for
> >> svn_cl__edit_file_externally should be explicit that the basename of
> >> PATH must be shell-safe?
> >
> > Probably wouldn't be a bad idea. I'll go ahead and add that in
> > later today.
> >
> >>
> >>> I've started working on the "launch external tool" callback, and I'm
> >>> wondering - does svn_path_shell_autoescape DTRT on Windows? With the
> >>> external tool callback we're going to have to either duplicate some
> >>> files or do some escaping so that things don't break when you work
> >>> with files that contain weird characters (spaces, etc) in the name.
> >>
> >> svn_path_shell_autoescape doesn't exist yet; I was suggesting that
> >> you
> >> (or somebody) write it. So I think it should DTRT on windows :-)
> >
> > Ahh. I'm not sure if I'm sadistic enough to figure out windows
> > escaping rules... ;-)
> >
> > Augie
> >
> >>
> >> --dave
> >>
> >> --
> >> David Glasser | glasser_at_mit.edu | http://www.davidglasser.net/
> >
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
> > For additional commands, e-mail: dev-help@subversion.tigris.org
> >
>
>

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Mon Aug 6 16:48:41 2007

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

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.