Author: jamessan
Date: Mon Feb 17 02:13:34 2020
New Revision: 1874093
URL: http://svn.apache.org/viewvc?rev=1874093&view=rev
Log:
Followup to r1874057, escape whitespace instead of quoting filename
As danielsh pointed out, only specific characters can be escaped by a backslash
in quoted shell strings. Rather than surrounding the escaped path with double
quotes, post-process the quoted path and manually escape any whitespace that is
present.
* subversion/libsvn_subr/cmdline.c
(escape_path): New function, wrapper around apr_pescape_shell(), which
handles escaping of whitespace.
(svn_cmdline__edit_file_externally, svn_cmdline__edit_string_externally):
Call the new function instead of apr_pescape_shell()
* subversion/tests/cmdline/update_tests.py
(update_accept_conflicts): Include ';' in renamed path ("p; i"), to ensure
non-whitespace escaping is handled correctly.
Modified:
subversion/trunk/subversion/libsvn_subr/cmdline.c
subversion/trunk/subversion/tests/cmdline/update_tests.py
Modified: subversion/trunk/subversion/libsvn_subr/cmdline.c
URL:
http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/cmdline.c?rev=1874093&r1=1874092&r2=1874093&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_subr/cmdline.c (original)
+++ subversion/trunk/subversion/libsvn_subr/cmdline.c Mon Feb 17 02:13:34 2020
@@ -1234,7 +1234,7 @@ svn_cmdline__be_interactive(svn_boolean_
}
-/* Helper for the next two functions. Set *EDITOR to some path to an
+/* Helper for the edit_externally functions. Set *EDITOR to some path to an
editor binary. Sources to search include: the EDITOR_CMD argument
(if not NULL), $SVN_EDITOR, the runtime CONFIG variable (if CONFIG
is not NULL), $VISUAL, $EDITOR. Return
@@ -1300,6 +1300,42 @@ find_editor_binary(const char **editor,
return SVN_NO_ERROR;
}
+/* Wrapper around apr_pescape_shell() which also escapes whitespace. */
+static const char *
+escape_path(apr_pool_t *pool, const char *orig_path)
+{
+ apr_size_t len, esc_len;
+ const char *path;
+ char *p, *esc_path;
+
+ path = apr_pescape_shell(pool, orig_path);
+
+ len = esc_len = 0;
+
+ /* Now that apr has done its escaping, we can check whether there's any
+ whitespace that also needs to be escaped. This must be done after the
+ fact, otherwise apr_pescape_shell() would escape the backslashes we're
+ inserting. */
+ for (p = (char *)path; *p; p++)
+ {
+ len++;
+ if (*p == ' ' || *p == '\t')
+ esc_len++;
+ }
+
+ if (esc_len == 0)
+ return path;
+
+ p = esc_path = apr_pcalloc(pool, len + esc_len + 1);
+ while (*path)
+ {
+ if (*path == ' ' || *path == '\t')
+ *p++ = '\\';
+ *p++ = *path++;
+ }
+
+ return esc_path;
+}
svn_error_t *
svn_cmdline__edit_file_externally(const char *path,
@@ -1333,8 +1369,7 @@ svn_cmdline__edit_file_externally(const
/* editor is explicitly documented as being interpreted by the user's shell,
and as such should already be quoted/escaped as needed. */
- cmd = apr_psprintf(pool, "%s \"%s\"", editor,
- apr_pescape_shell(pool, file_name));
+ cmd = apr_psprintf(pool, "%s %s", editor, escape_path(pool, file_name));
sys_err = system(cmd);
apr_err = apr_filepath_set(old_cwd, pool);
@@ -1496,8 +1531,7 @@ svn_cmdline__edit_string_externally(svn_
/* editor is explicitly documented as being interpreted by the user's shell,
and as such should already be quoted/escaped as needed. */
- cmd = apr_psprintf(pool, "%s \"%s\"", editor,
- apr_pescape_shell(pool, tmpfile_native));
+ cmd = apr_psprintf(pool, "%s %s", editor, escape_path(pool, tmpfile_native));
/* If the caller wants us to leave the file around, return the path
of the file we'll use, and make a note not to destroy it. */
Modified: subversion/trunk/subversion/tests/cmdline/update_tests.py
URL:
http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/cmdline/update_tests.py?rev=1874093&r1=1874092&r2=1874093&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/cmdline/update_tests.py (original)
+++ subversion/trunk/subversion/tests/cmdline/update_tests.py Mon Feb 17
02:13:34 2020
@@ -3649,12 +3649,12 @@ def update_accept_conflicts(sbox):
alpha_path = sbox.ospath('A/B/E/alpha')
beta_path = sbox.ospath('A/B/E/beta')
pi_path = sbox.ospath('A/D/G/pi')
- p_i_path = sbox.ospath('A/D/G/p i')
+ p_i_path = sbox.ospath('A/D/G/p; i')
rho_path = sbox.ospath('A/D/G/rho')
- # Rename pi to "p i" so we can exercise SVN_EDITOR's handling of paths with
- # whitespace
- sbox.simple_move('A/D/G/pi', 'A/D/G/p i')
+ # Rename pi to "p; i" so we can exercise SVN_EDITOR's handling of paths with
+ # special characters
+ sbox.simple_move('A/D/G/pi', 'A/D/G/p; i')
sbox.simple_commit()
sbox.simple_update()
@@ -3676,7 +3676,7 @@ def update_accept_conflicts(sbox):
mu_path_backup = os.path.join(wc_backup, 'A', 'mu')
alpha_path_backup = os.path.join(wc_backup, 'A', 'B', 'E', 'alpha')
beta_path_backup = os.path.join(wc_backup, 'A', 'B', 'E', 'beta')
- p_i_path_backup = os.path.join(wc_backup, 'A', 'D', 'G', 'p i')
+ p_i_path_backup = os.path.join(wc_backup, 'A', 'D', 'G', 'p; i')
rho_path_backup = os.path.join(wc_backup, 'A', 'D', 'G', 'rho')
svntest.main.file_append(iota_path_backup,
'My appended text for iota\n')
@@ -3700,7 +3700,7 @@ def update_accept_conflicts(sbox):
'A/mu' : Item(verb='Sending'),
'A/B/E/alpha': Item(verb='Sending'),
'A/B/E/beta': Item(verb='Sending'),
- 'A/D/G/p i' : Item(verb='Sending'),
+ 'A/D/G/p; i' : Item(verb='Sending'),
'A/D/G/rho' : Item(verb='Sending'),
})
@@ -3710,8 +3710,8 @@ def update_accept_conflicts(sbox):
expected_status.tweak('A/mu', wc_rev=3)
expected_status.tweak('A/B/E/alpha', wc_rev=3)
expected_status.tweak('A/B/E/beta', wc_rev=3)
- expected_status.rename({'A/D/G/pi': 'A/D/G/p i'})
- expected_status.tweak('A/D/G/p i', wc_rev=3)
+ expected_status.rename({'A/D/G/pi': 'A/D/G/p; i'})
+ expected_status.tweak('A/D/G/p; i', wc_rev=3)
expected_status.tweak('A/D/G/rho', wc_rev=3)
# Commit.
@@ -3806,15 +3806,15 @@ def update_accept_conflicts(sbox):
'My appended text for alpha\n'))
expected_disk.tweak('A/B/E/beta', contents=("This is the file 'beta'.\n"
'Their appended text for
beta\n'))
- expected_disk.rename({'A/D/G/pi': 'A/D/G/p i'})
- expected_disk.tweak('A/D/G/p i', contents=("This is the file 'pi'.\n"
- '<<<<<<< .mine\n'
- 'My appended text for pi\n'
- '||||||| .r2\n'
- '=======\n'
- 'Their appended text for pi\n'
- '>>>>>>> .r3\n'
- 'foo\n'))
+ expected_disk.rename({'A/D/G/pi': 'A/D/G/p; i'})
+ expected_disk.tweak('A/D/G/p; i', contents=("This is the file 'pi'.\n"
+ '<<<<<<< .mine\n'
+ 'My appended text for pi\n'
+ '||||||| .r2\n'
+ '=======\n'
+ 'Their appended text for pi\n'
+ '>>>>>>> .r3\n'
+ 'foo\n'))
expected_disk.tweak('A/D/G/rho', contents=("This is the file 'rho'.\n"
'<<<<<<< .mine\n'
'My appended text for rho\n'
@@ -3831,16 +3831,16 @@ def update_accept_conflicts(sbox):
# Set the expected status for the test
expected_status = svntest.actions.get_virginal_state(wc_backup, 3)
- expected_status.rename({'A/D/G/pi': 'A/D/G/p i'})
+ expected_status.rename({'A/D/G/pi': 'A/D/G/p; i'})
expected_status.tweak('iota', 'A/B/lambda', 'A/mu',
'A/B/E/alpha', 'A/B/E/beta',
- 'A/D/G/p i', 'A/D/G/rho', wc_rev=3)
+ 'A/D/G/p; i', 'A/D/G/rho', wc_rev=3)
expected_status.tweak('iota', status='C ')
expected_status.tweak('A/B/lambda', status='C ')
expected_status.tweak('A/mu', status='M ')
expected_status.tweak('A/B/E/alpha', status='M ')
expected_status.tweak('A/B/E/beta', status=' ')
- expected_status.tweak('A/D/G/p i', status='M ')
+ expected_status.tweak('A/D/G/p; i', status='M ')
expected_status.tweak('A/D/G/rho', status='C ')
# Set the expected output for the test
