From 0e569fd4bfc661f922f48bb65b2579d98f5e8ef3 Mon Sep 17 00:00:00 2001 From: Peter Grayson Date: Mon, 1 Feb 2021 17:38:19 -0500 Subject: [PATCH] Avoid importing invalid and duplicate patch names This aims to be a comprehensive repair of issues with `stg import` allowing invalid and duplicate patch names. The new Patches.make_name() method creates a unique and valid patch name based on its knowledge of existing patch names. Since StGit patch names are used as Git refs, make_name() also attempts to enforce Git's rules for ref naming. Generated patch names are checked with `git check-ref-format` before being returned. Repairs #64 Signed-off-by: Peter Grayson --- stgit/commands/imprt.py | 73 +++++++++++++--------------- stgit/config.py | 1 + stgit/lib/stack.py | 76 ++++++++++++++++++++++++++++++ stgit/utils.py | 2 - t/t1800-import.sh | 12 ++++- t/t1801-import-email.sh | 8 ++++ t/t1801/email-mbox-same-subject | 102 ++++++++++++++++++++++++++++++++++++++++ 7 files changed, 230 insertions(+), 44 deletions(-) create mode 100644 t/t1801/email-mbox-same-subject diff --git a/stgit/commands/imprt.py b/stgit/commands/imprt.py index 1e531a3..48aa827 100644 --- a/stgit/commands/imprt.py +++ b/stgit/commands/imprt.py @@ -18,7 +18,6 @@ from stgit.lib.git import CommitData, Date, Person from stgit.lib.transaction import StackTransaction, TransactionHalted from stgit.out import out from stgit.run import Run -from stgit.utils import make_patch_name __copyright__ = """ Copyright (C) 2005, Catalin Marinas @@ -149,17 +148,6 @@ options = ( directory = DirectoryHasRepository() -def __strip_patch_name(name): - stripped = re.sub('^[0-9]+-(.*)$', r'\g<1>', name) - stripped = re.sub(r'^(.*)\.(diff|patch)$', r'\g<1>', stripped) - return stripped - - -def __replace_slashes_with_dashes(name): - stripped = name.replace('/', '-') - return stripped - - def __create_patch( filename, message, author_name, author_email, author_date, diff, options ): @@ -168,29 +156,31 @@ def __create_patch( if options.name: name = options.name - if not stack.patches.is_name_valid(name): - raise CmdException('Invalid patch name: %s' % name) elif filename: name = os.path.basename(filename) else: name = '' - if options.stripname: - name = __strip_patch_name(name) - if not name: - if options.ignore or options.replace: + if options.stripname: + # Removing leading numbers and trailing extension + name = re.sub( + r'''^ + (?:[0-9]+-)? # Optional leading patch number + (.*?) # Patch name group (non-greedy) + (?:\.(?:diff|patch))? # Optional .diff or .patch extension + $ + ''', + r'\g<1>', + name, + flags=re.VERBOSE, + ) - def unacceptable_name(name): - return False + need_unique = not (options.ignore or options.replace) - else: - unacceptable_name = stack.patches.exists - name = make_patch_name(message, unacceptable_name) + if name: + name = stack.patches.make_name(name, unique=need_unique, lower=False) else: - # fix possible invalid characters in the patch name - name = re.sub(r'[^\w.]+', '-', name).strip('-') - - assert stack.patches.is_name_valid(name) + name = stack.patches.make_name(message, unique=need_unique, lower=True) if options.ignore and name in stack.patchorder.applied: out.info('Ignoring already applied patch "%s"' % name) @@ -266,28 +256,30 @@ def __get_handle_and_name(filename): return (open(filename, 'rb'), filename) -def __import_file(filename, options, patch=None): +def __import_file(filename, options): """Import a patch from a file or standard input""" - pname = None if filename: - (f, pname) = __get_handle_and_name(filename) + f, filename = __get_handle_and_name(filename) else: f = os.fdopen(sys.__stdin__.fileno(), 'rb') - if patch: - pname = patch - elif not pname: - pname = filename - - (message, author_name, author_email, author_date, diff) = parse_patch( - f.read(), contains_diff=True - ) + patch_data = f.read() if filename: f.close() + message, author_name, author_email, author_date, diff = parse_patch( + patch_data, contains_diff=True + ) + __create_patch( - pname, message, author_name, author_email, author_date, diff, options + filename, + message, + author_name, + author_email, + author_date, + diff, + options, ) @@ -327,9 +319,8 @@ def __import_series(filename, options): else: options.strip = 1 patchfile = os.path.join(patchdir, patch) - patch = __replace_slashes_with_dashes(patch) - __import_file(patchfile, options, patch) + __import_file(patchfile, options) if filename: f.close() diff --git a/stgit/config.py b/stgit/config.py index 706d0bb..226698a 100644 --- a/stgit/config.py +++ b/stgit/config.py @@ -31,6 +31,7 @@ DEFAULTS = [ ('stgit.autoimerge', ['no']), ('stgit.fetchcmd', ['git fetch']), ('stgit.keepoptimized', ['no']), + ('stgit.namelength', ['30']), ('stgit.pager', ['less']), ('stgit.pick.expose-format', ['format:%B%n(imported from commit %H)']), ('stgit.pull-policy', ['pull']), diff --git a/stgit/lib/stack.py b/stgit/lib/stack.py index 17572d4..1ca97b0 100644 --- a/stgit/lib/stack.py +++ b/stgit/lib/stack.py @@ -1,4 +1,5 @@ """A Python class hierarchy wrapping the StGit on-disk metadata.""" +import re from stgit.config import config from stgit.exception import StackException @@ -167,6 +168,81 @@ class Patches: self._patches[name] = p return p + def make_name(self, raw, unique=True, lower=True): + """Make a unique and valid patch name from provided raw name. + + The raw name may come from a filename, commit message, or email subject line. + + The generated patch name will meet the rules of `git check-ref-format` along + with some additional StGit patch name rules. + + """ + default_name = 'patch' + + for line in raw.split('\n'): + if line: + break + + if not line: + line = default_name + + if lower: + line = line.lower() + + parts = [] + for part in line.split('/'): + # fmt: off + part = re.sub(r'\.lock$', '', part) # Disallowed in Git refs + part = re.sub(r'^\.+|\.+$', '', part) # Cannot start or end with '.' + part = re.sub(r'\.+', '.', part) # No consecutive '.' + part = re.sub(r'[^\w.]+', '-', part) # Non-word and whitespace to dashes + part = re.sub(r'-+', '-', part) # Squash consecutive dashes + part = re.sub(r'^-+|-+$', '', part) # Remove leading and trailing dashes + # fmt: on + if part: + parts.append(part) + + long_name = '/'.join(parts) + + # TODO: slashes could be allowed in the future. + long_name = long_name.replace('/', '-') + + if not long_name: + long_name = default_name + + assert self.is_name_valid(long_name) + + name_len = config.getint('stgit.namelength') + + words = long_name.split('-') + short_name = words[0] + for word in words[1:]: + new_name = '%s-%s' % (short_name, word) + if name_len <= 0 or len(new_name) <= name_len: + short_name = new_name + else: + break + assert self.is_name_valid(short_name) + + if not unique: + return short_name + + unique_name = short_name + while self.exists(unique_name): + m = re.match(r'(.*?)(-)?(\d+)$', unique_name) + if m: + base, sep, n_str = m.groups() + n = int(n_str) + 1 + if sep: + unique_name = '%s%s%d' % (base, sep, n) + else: + unique_name = '%s%d' % (base, n) + else: + unique_name = '%s-1' % unique_name + + assert self.is_name_valid(unique_name) + return unique_name + class Stack(Branch): """Represents a StGit stack. diff --git a/stgit/utils.py b/stgit/utils.py index a63056e..44b5fcb 100644 --- a/stgit/utils.py +++ b/stgit/utils.py @@ -144,8 +144,6 @@ def patch_name_from_msg(msg): return None name_len = config.getint('stgit.namelength') - if not name_len: - name_len = 30 subject_line = msg.split('\n', 1)[0].lstrip().lower() words = re.sub(r'(?u)[\W]+', ' ', subject_line).split() diff --git a/t/t1800-import.sh b/t/t1800-import.sh index 5cc311c..7f4c272 100755 --- a/t/t1800-import.sh +++ b/t/t1800-import.sh @@ -32,6 +32,16 @@ test_expect_success \ ' test_expect_success \ + 'Attempt import same patch twice' \ + ' + stg import "$TEST_DIRECTORY"/t1800/git-diff && + stg pop && + stg import "$TEST_DIRECTORY"/t1800/git-diff && + test "$(echo $(stg series --noprefix))" = "git-diff-1 git-diff" && + stg delete .. + ' + +test_expect_success \ 'Apply a patch and edit message' \ ' test_set_editor "$(pwd)/fake-editor" && @@ -227,7 +237,7 @@ test_expect_success \ ' cat some.patch | stg import --ignore --author "Some Author " && - stg top | grep -e "unknown" && + stg top | grep -e "patch" && stg show | grep -e "Author: Some Author " && stg delete --top ' diff --git a/t/t1801-import-email.sh b/t/t1801-import-email.sh index afc637c..157036d 100755 --- a/t/t1801-import-email.sh +++ b/t/t1801-import-email.sh @@ -106,6 +106,14 @@ test_expect_success \ ' test_expect_success \ + 'Import patches from mbox with duplicate subjects' \ + ' + stg import -M "$TEST_DIRECTORY"/t1801/email-mbox-same-subject && + test "$(echo $(stg series --noprefix --applied))" = "my-patch my-patch-1 my-patch-2" && + stg delete .. + ' + +test_expect_success \ 'Apply several patches from an mbox file from stdin' \ ' cat "$TEST_DIRECTORY"/t1801/email-mbox | stg import -M && diff --git a/t/t1801/email-mbox-same-subject b/t/t1801/email-mbox-same-subject new file mode 100644 index 0000000..05ba9e0 --- /dev/null +++ b/t/t1801/email-mbox-same-subject @@ -0,0 +1,102 @@ +From nobody Sat Nov 11 12:45:27 2006 +From: Inge =?utf-8?q?Str=C3=B6m?= +Subject: My Patch +To: Upstream +Date: Sat, 11 Nov 2006 12:45:27 +0100 +Message-ID: <20061111114527.31778.12942.stgit@localhost> +User-Agent: StGIT/0.11 +MIME-Version: 1.0 +Content-Type: text/plain; charset="utf-8" +Content-Transfer-Encoding: 8bit +Status: RO +Content-Length: 304 +Lines: 19 + +Signed-off-by: Inge Ström +--- + + foo.txt | 2 +- + 1 files changed, 1 insertions(+), 1 deletions(-) + +diff --git a/foo.txt b/foo.txt +index ad01662..91527b1 100644 +--- a/foo.txt ++++ b/foo.txt +@@ -7,7 +7,7 @@ dobidum + dobodam + dobodim + dobodum +-dibedam ++dibedad + dibedim + dibedum + dibidam + +From nobody Sat Nov 11 12:45:27 2006 +From: Inge =?utf-8?q?Str=C3=B6m?= +Subject: My Patch +To: Upstream +Date: Sat, 11 Nov 2006 12:45:27 +0100 +Message-ID: <20061111114527.31778.92851.stgit@localhost> +In-Reply-To: <20061111114527.31778.12942.stgit@localhost> +References: <20061111114527.31778.12942.stgit@localhost> +User-Agent: StGIT/0.11 +MIME-Version: 1.0 +Content-Type: text/plain; charset="utf-8" +Content-Transfer-Encoding: 8bit +Status: RO +Content-Length: 296 +Lines: 18 + +Signed-off-by: Inge Ström +--- + + foo.txt | 1 + + 1 files changed, 1 insertions(+), 0 deletions(-) + +diff --git a/foo.txt b/foo.txt +index 91527b1..79922d7 100644 +--- a/foo.txt ++++ b/foo.txt +@@ -18,6 +18,7 @@ dibodim + dibodum + dabedam + dabedim ++dibedam + dabedum + dabidam + dabidim + +From nobody Sat Nov 11 12:45:27 2006 +From: Inge =?utf-8?q?Str=C3=B6m?= +Subject: My Patch +To: Upstream +Date: Sat, 11 Nov 2006 12:45:27 +0100 +Message-ID: <20061111114527.31778.45876.stgit@localhost> +In-Reply-To: <20061111114527.31778.12942.stgit@localhost> +References: <20061111114527.31778.12942.stgit@localhost> +User-Agent: StGIT/0.11 +MIME-Version: 1.0 +Content-Type: text/plain; charset="utf-8" +Content-Transfer-Encoding: 8bit +Status: RO +Content-Length: 278 +Lines: 16 + +Signed-off-by: Inge Ström +--- + + foo.txt | 1 - + 1 files changed, 0 insertions(+), 1 deletions(-) + +diff --git a/foo.txt b/foo.txt +index 79922d7..6f978b4 100644 +--- a/foo.txt ++++ b/foo.txt +@@ -24,5 +24,4 @@ dabidam + dabidim + dabidum + dabodam +-dabodim + dabodum + -- 2.11.4.GIT