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 Thu Jul 26 03:29:37 2007