Check save/split names early via valid_save_name

Previously, split and save would just wait to see if the final
update_ref failed.  Instead, check the validity of the name early,
before doing any significant work.

Thanks to vi0oss for reporting the problem.

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

diff --git a/cmd/save-cmd.py b/cmd/save-cmd.py
index 56351efd66cd84100b41bd969124a9ff6edaaacb..f06e3a46d2b0933260cc52fb9220f10fe9f0bc5a 100755 (executable)
--- a/cmd/save-cmd.py
+++ b/cmd/save-cmd.py
@@ -15,7 +15,7 @@ from bup.helpers import (add_error, grafted_path_components, handle_ctrl_c,
                          hostname, istty2, log, parse_date_or_fatal, parse_num,
                          path_components, progress, qprogress, resolve_parent,
                          saved_errors, stripped_path_components,
-                         userfullname, username)
+                         userfullname, username, valid_save_name)
 
 
 optspec = """
@@ -81,7 +81,7 @@ is_reverse = os.environ.get('BUP_SERVER_REVERSE')
 if is_reverse and opt.remote:
     o.fatal("don't use -r in reverse mode; it's automatic")
 
-if opt.name and opt.name.startswith('.'):
+if opt.name and not valid_save_name(opt.name):
     o.fatal("'%s' is not a valid branch name" % opt.name)
 refname = opt.name and 'refs/heads/%s' % opt.name or None
 if opt.remote or is_reverse:

diff --git a/cmd/split-cmd.py b/cmd/split-cmd.py
index 57c7392c78a57864d579e948ad42d5c3bf78f978..637bbd9af6ca593a19eef6b5d17c2ed8001ca46d 100755 (executable)
--- a/cmd/split-cmd.py
+++ b/cmd/split-cmd.py
@@ -10,7 +10,7 @@ import os, sys, time
 from bup import hashsplit, git, options, client
 from bup.helpers import (add_error, handle_ctrl_c, hostname, log, parse_num,
                          qprogress, reprogress, saved_errors,
-                         userfullname, username)
+                         userfullname, username, valid_save_name)
 
 
 optspec = """
@@ -89,7 +89,7 @@ if is_reverse and opt.remote:
     o.fatal("don't use -r in reverse mode; it's automatic")
 start_time = time.time()
 
-if opt.name and opt.name.startswith('.'):
+if opt.name and not valid_save_name(opt.name):
     o.fatal("'%s' is not a valid branch name." % opt.name)
 refname = opt.name and 'refs/heads/%s' % opt.name or None
 if opt.noop or opt.copy:

diff --git a/lib/bup/helpers.py b/lib/bup/helpers.py
index b052b63021511b035a94ac9945e01dce1099637d..581a4bb3827465ab983f7afe4ba1602b44357870 100644 (file)
--- a/lib/bup/helpers.py
+++ b/lib/bup/helpers.py
@@ -1079,3 +1079,22 @@ else:
         return time.strftime('%z', localtime(t))
     def to_py_time(x):
         return x
+
+
+_some_invalid_save_parts_rx = re.compile(r'[[ ~^:?*\\]|\.\.|//|@{')
+
+def valid_save_name(name):
+    # Enforce a superset of the restrictions in git-check-ref-format(1)
+    if name == '@' \
+       or name.startswith('/') or name.endswith('/') \
+       or name.endswith('.'):
+        return False
+    if _some_invalid_save_parts_rx.search(name):
+        return False
+    for c in name:
+        if ord(c) < 0x20 or ord(c) == 0x7f:
+            return False
+    for part in name.split('/'):
+        if part.startswith('.') or part.endswith('.lock'):
+            return False
+    return True

diff --git a/lib/bup/t/thelpers.py b/lib/bup/t/thelpers.py
index 8c02a110b0a8a57ca059a39fa54b06cff1d444e8..9d23eae1facd8c0c7e3234674fa09d2763f289be 100644 (file)
--- a/lib/bup/t/thelpers.py
+++ b/lib/bup/t/thelpers.py
@@ -221,3 +221,29 @@ def test_utc_offset_str():
                     del os.environ['TZ']
                 except KeyError:
                     pass
+
+@wvtest
+def test_valid_save_name():
+    with no_lingering_errors():
+        valid = helpers.valid_save_name
+        WVPASS(valid('x'))
+        WVPASS(valid('x@'))
+        WVFAIL(valid('@'))
+        WVFAIL(valid('/'))
+        WVFAIL(valid('/foo'))
+        WVFAIL(valid('foo/'))
+        WVFAIL(valid('/foo/'))
+        WVFAIL(valid('foo//bar'))
+        WVFAIL(valid('.'))
+        WVFAIL(valid('bar.'))
+        WVFAIL(valid('foo@{'))
+        for x in ' ~^:?*[\\':
+            WVFAIL(valid('foo' + x))
+        for i in range(20):
+            WVFAIL(valid('foo' + chr(i)))
+        WVFAIL(valid('foo' + chr(0x7f)))
+        WVFAIL(valid('foo..bar'))
+        WVFAIL(valid('bar.lock/baz'))
+        WVFAIL(valid('foo/bar.lock/baz'))
+        WVFAIL(valid('.bar/baz'))
+        WVFAIL(valid('foo/.bar/baz'))