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.
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.
Peace,
Augie
[[[
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 25820)
+++ subversion/svn/util.c (working copy)
@@ -149,14 +149,40 @@
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 10:49 AM, David Glasser wrote:
> On 7/25/07, Augie Fackler <durin42@gmail.com> wrote:
>> + cmd = apr_psprintf(pool, "%s \'%s\'", editor, file_name);
>
> Hmm. It does feel to me that it is *slightly* more likely that a user
> file name would have a single quote in it ("David's version") than a
> double quote. (And if you are using single quotes, you don't need the
> backslashes there.)
>
> Or really, assuming we're stuck with using "system" (which seems to be
> the case) we might want to add an svn_path_shell_autoescape, which is
> like svn_path_uri_autoescape but escapes with backslashes instead of
> %nn codes... Not only would that let you edit files with weird names,
> but you wouldn't need all of the directory-changing games in the first
> place.
>
> --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
Received on Wed Jul 25 18:04:04 2007