{"thread":{"id":"25356","subject":"[stgit PATCH] commands.{new,rename}: verify patch names","startedAt":"2010-10-05T11:45:41Z","lastAt":"2010-10-05T12:52:25Z","messageCount":3,"participants":["Max Kellermann","Gustav Hållberg"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"152667","messageId":"20101005114541.15037.53617.stgit@woodpecker.blarg.de","threadId":"25356","inReplyTo":null,"subject":"[stgit PATCH] commands.{new,rename}: verify patch names","fromName":"Max Kellermann","fromEmail":"max@duempel.org","sentAt":"2010-10-05T11:45:41Z","receivedAt":"2010-10-05T11:45:41Z","isPatch":true,"sender":{"key":"max@duempel.org","avatar":null},"body":"Don't allow patches with invalid names.  For example, a patch with a\nslash in the name will cause the underlying git command to fail, and\nstgit doesn't handle this error condition properly.\n---\n stgit/commands/new.py    |    3 +++\n stgit/commands/rename.py |    3 +++\n stgit/utils.py           |    5 +++++\n 3 files changed, 11 insertions(+), 0 deletions(-)\n\ndiff --git a/stgit/commands/new.py b/stgit/commands/new.py\nindex d5c5382..6bd7314 100644\n--- a/stgit/commands/new.py\n+++ b/stgit/commands/new.py\n@@ -61,6 +61,9 @@ def func(parser, options, args):\n         name = args[0]\n         if stack.patches.exists(name):\n             raise common.CmdException('%s: patch already exists' % name)\n+\n+        if not utils.check_patch_name(name):\n+            raise common.CmdException('%s: invalid patch name' % name)\n     else:\n         parser.error('incorrect number of arguments')\n \ndiff --git a/stgit/commands/rename.py b/stgit/commands/rename.py\nindex db898cb..7c229be 100644\n--- a/stgit/commands/rename.py\n+++ b/stgit/commands/rename.py\n@@ -51,6 +51,9 @@ def func(parser, options, args):\n     else:\n         parser.error('incorrect number of arguments')\n \n+    if not check_patch_name(new):\n+        raise CmdException('%s: invalid patch name' % new)\n+\n     out.start('Renaming patch \"%s\" to \"%s\"' % (old, new))\n     crt_series.rename_patch(old, new)\n \ndiff --git a/stgit/utils.py b/stgit/utils.py\nindex 2955adf..5c64871 100644\n--- a/stgit/utils.py\n+++ b/stgit/utils.py\n@@ -241,6 +241,11 @@ def make_patch_name(msg, unacceptable, default_name = 'patch'):\n         patchname = default_name\n     return find_patch_name(patchname, unacceptable)\n \n+def check_patch_name(name):\n+    \"\"\"Checks if the specified name is a valid patch name. For\n+    technical reasons, we cannot allow a slash and other characters.\"\"\"\n+    return len(name) > 0 and name[0] != '.' and re.search(r'[\\x00-\\x20]', name) is None\n+\n # any and all functions are builtin in Python 2.5 and higher, but not\n # in 2.4.\n if not 'any' in dir(__builtins__):\n"},{"id":"152669","messageId":"AANLkTin9PyfY+-1=mJKMZa2FJ5YC2D27iPtiocCWY+eP@mail.gmail.com","threadId":"25356","inReplyTo":"20101005114541.15037.53617.stgit@woodpecker.blarg.de","subject":"Re: [stgit PATCH] commands.{new,rename}: verify patch names","fromName":"Gustav Hållberg","fromEmail":"gustav@gmail.com","sentAt":"2010-10-05T12:44:43Z","receivedAt":"2010-10-05T12:44:43Z","isPatch":true,"sender":{"key":"gustav@gmail.com","avatar":null},"body":"On Tue, Oct 5, 2010 at 1:45 PM, Max Kellermann <max@duempel.org> wrote:\n> +def check_patch_name(name):\n> +    \"\"\"Checks if the specified name is a valid patch name. For\n> +    technical reasons, we cannot allow a slash and other characters.\"\"\"\n> +    return len(name) > 0 and name[0] != '.' and re.search(r'[\\x00-\\x20]', name) is None\n\nI don't quite understand how the above would filter out slashes.\n\nThere are also other types of names that won't work correctly in git,\nsuch as names starting with (double?) hyphens.\n\nDoes anyone know if it's explicitly documented anywhere which types of\nnames are (meant to be) allowed for git refs?\nNote that you can create refs with names that don't actually work\ncorrectly; e.g.,\n\n sh$ git tag -- --foo\n sh$ git rev-parse --foo\n <failure>\n\n- Gustav\n"},{"id":"152671","messageId":"20101005125225.GA12416@mail.blarg.de","threadId":"25356","inReplyTo":"AANLkTin9PyfY+-1=mJKMZa2FJ5YC2D27iPtiocCWY+eP@mail.gmail.com","subject":"Re: [stgit PATCH] commands.{new,rename}: verify patch names","fromName":"Max Kellermann","fromEmail":"max@duempel.org","sentAt":"2010-10-05T12:52:25Z","receivedAt":"2010-10-05T12:52:25Z","isPatch":true,"sender":{"key":"max@duempel.org","avatar":null},"body":"On 2010/10/05 14:44, Gustav Hållberg <gustav@gmail.com> wrote:\n> On Tue, Oct 5, 2010 at 1:45 PM, Max Kellermann <max@duempel.org> wrote:\n> > +def check_patch_name(name):\n> > +    \"\"\"Checks if the specified name is a valid patch name. For\n> > +    technical reasons, we cannot allow a slash and other characters.\"\"\"\n> > +    return len(name) > 0 and name[0] != '.' and re.search(r'[\\x00-\\x20]', name) is None\n> \n> I don't quite understand how the above would filter out slashes.\n\nOh damn, you're right.  I had slashes explicitly forbidden in a\nprevious revision of my patch, that got lost when I added my \"kill all\nwhitespace\" change.  I'll resubmit.\n\n>  sh$ git tag -- --foo\n>  sh$ git rev-parse --foo\n>  <failure>\n\nI guess this is a problem because \"git-rev-parse\" doesn't follow the\nconvention of the \"--\" option separator.\n"}]}