Rework shstr to handle bytes and strings; add squote and bquote

These could be smarter, i.e '1 could become "'"1 rather than ''"'"'1',
but we can always improve it later.  And add at last some tests.
Don't rely on compat.quote for strings so that we know we'll have the
same behavior for bytes and strings.

Thanks to Johannes Berg for pointing out an incorrect variable name in
a previous revision.

Signed-off-by: Rob Browning <rlb@defaultvalue.org>
Tested-by: Rob Browning <rlb@defaultvalue.org>

diff --git a/lib/bup/helpers.py b/lib/bup/helpers.py
index 84b1b978682f375c6bbeaeaa4b7fbde53380f51e..205343988b4d940477bd834aa834401d09c1e56e 100644 (file)
--- a/lib/bup/helpers.py
+++ b/lib/bup/helpers.py
@@ -6,7 +6,6 @@ from contextlib import contextmanager
 from ctypes import sizeof, c_void_p
 from math import floor
 from os import environ
-from pipes import quote
 from subprocess import PIPE, Popen
 import sys, os, pwd, subprocess, errno, socket, select, mmap, stat, re, struct
 import hashlib, heapq, math, operator, time, grp, tempfile
@@ -268,11 +267,47 @@ def unlink(f):
             raise
 
 
+_bq_simple_id_rx = re.compile(br'^[-_./a-zA-Z0-9]+$')
+_sq_simple_id_rx = re.compile(r'^[-_./a-zA-Z0-9]+$')
+
+def bquote(x):
+    if x == b'':
+        return b"''"
+    if _bq_simple_id_rx.match(x):
+        return x
+    return b"'%s'" % x.replace(b"'", b"'\"'\"'")
+
+def squote(x):
+    if x == '':
+        return "''"
+    if _sq_simple_id_rx.match(x):
+        return x
+    return "'%s'" % x.replace("'", "'\"'\"'")
+
+def quote(x):
+    if isinstance(x, bytes):
+        return bquote(x)
+    if isinstance(x, compat.str_type):
+        return squote(x)
+    assert False
+
 def shstr(cmd):
-    if isinstance(cmd, compat.str_type):
+    """Return a shell quoted string for cmd if it's a sequence, else cmd.
+
+    cmd must be a string, bytes, or a sequence of one or the other,
+    and the assumption is that if cmd is a string or bytes, then it's
+    already quoted (because it's what's actually being passed to
+    call() and friends.  e.g. log(shstr(cmd)); call(cmd)
+
+    """
+    if isinstance(cmd, (bytes, compat.str_type)):
         return cmd
-    else:
-        return ' '.join(map(quote, cmd))
+    elif all(isinstance(x, bytes) for x in cmd):
+        return b' '.join(map(bquote, cmd))
+    elif all(isinstance(x, compat.str_type) for x in cmd):
+        return ' '.join(map(squote, cmd))
+    raise TypeError('unsupported shstr argument: ' + repr(cmd))
+
 
 exc = subprocess.check_call
 

diff --git a/lib/bup/t/thelpers.py b/lib/bup/t/thelpers.py
index 428970352716e593dcaf7cb20d4627a3963b2540..c71cbb71a448b71257aec81649c583bda34d5ab6 100644 (file)
--- a/lib/bup/t/thelpers.py
+++ b/lib/bup/t/thelpers.py
@@ -9,6 +9,7 @@ from bup.compat import bytes_from_byte, bytes_from_uint, environ
 from bup.helpers import (atomically_replaced_file, batchpipe, detect_fakeroot,
                          grafted_path_components, mkdirp, parse_num,
                          path_components, readpipe, stripped_path_components,
+                         shstr,
                          utc_offset_str)
 from buptest import no_lingering_errors, test_tempdir
 import bup._helpers as _helpers
@@ -102,6 +103,28 @@ def test_grafted_path_components():
         WVEXCEPT(Exception, grafted_path_components, b'foo', [])
 
 
+@wvtest
+def test_shstr():
+    with no_lingering_errors():
+        # Do nothing for strings and bytes
+        WVPASSEQ(shstr(b''), b'')
+        WVPASSEQ(shstr(b'1'), b'1')
+        WVPASSEQ(shstr(b'1 2'), b'1 2')
+        WVPASSEQ(shstr(b"1'2"), b"1'2")
+        WVPASSEQ(shstr(''), '')
+        WVPASSEQ(shstr('1'), '1')
+        WVPASSEQ(shstr('1 2'), '1 2')
+        WVPASSEQ(shstr("1'2"), "1'2")
+
+        # Escape parts of sequences
+        WVPASSEQ(shstr((b'1 2', b'3')), b"'1 2' 3")
+        WVPASSEQ(shstr((b"1'2", b'3')), b"'1'\"'\"'2' 3")
+        WVPASSEQ(shstr((b"'1", b'3')), b"''\"'\"'1' 3")
+        WVPASSEQ(shstr(('1 2', '3')), "'1 2' 3")
+        WVPASSEQ(shstr(("1'2", '3')), "'1'\"'\"'2' 3")
+        WVPASSEQ(shstr(("'1", '3')), "''\"'\"'1' 3")
+
+
 @wvtest
 def test_readpipe():
     with no_lingering_errors():