[stgit PATCH] commands.{new,rename}: verify patch names

[stgit PATCH] commands.{new,rename}: verify patch names

[Max Kellermann]

Don't allow patches with invalid names.  For example, a patch with a
slash in the name will cause the underlying git command to fail, and
stgit doesn't handle this error condition properly.
---
 stgit/commands/new.py    |    3 +++
 stgit/commands/rename.py |    3 +++
 stgit/utils.py           |    5 +++++
 3 files changed, 11 insertions(+), 0 deletions(-)

diff --git a/stgit/commands/new.py b/stgit/commands/new.py
index d5c5382..6bd7314 100644
--- a/stgit/commands/new.py
+++ b/stgit/commands/new.py
@@ -61,6 +61,9 @@ def func(parser, options, args):
         name = args[0]
         if stack.patches.exists(name):
             raise common.CmdException('%s: patch already exists' % name)
+
+        if not utils.check_patch_name(name):
+            raise common.CmdException('%s: invalid patch name' % name)
     else:
         parser.error('incorrect number of arguments')
 
diff --git a/stgit/commands/rename.py b/stgit/commands/rename.py
index db898cb..7c229be 100644
--- a/stgit/commands/rename.py
+++ b/stgit/commands/rename.py
@@ -51,6 +51,9 @@ def func(parser, options, args):
     else:
         parser.error('incorrect number of arguments')
 
+    if not check_patch_name(new):
+        raise CmdException('%s: invalid patch name' % new)
+
     out.start('Renaming patch "%s" to "%s"' % (old, new))
     crt_series.rename_patch(old, new)
 
diff --git a/stgit/utils.py b/stgit/utils.py
index 2955adf..5c64871 100644
--- a/stgit/utils.py
+++ b/stgit/utils.py
@@ -241,6 +241,11 @@ def make_patch_name(msg, unacceptable, default_name = 'patch'):
         patchname = default_name
     return find_patch_name(patchname, unacceptable)
 
+def check_patch_name(name):
+    """Checks if the specified name is a valid patch name. For
+    technical reasons, we cannot allow a slash and other characters."""
+    return len(name) > 0 and name[0] != '.' and re.search(r'[\x00-\x20]', name) is None
+
 # any and all functions are builtin in Python 2.5 and higher, but not
 # in 2.4.
 if not 'any' in dir(__builtins__):

Re: [stgit PATCH] commands.{new,rename}: verify patch names

[Gustav Hållberg]

On Tue, Oct 5, 2010 at 1:45 PM, Max Kellermann <max@duempel.org> wrote:
> +def check_patch_name(name):
> +    """Checks if the specified name is a valid patch name. For
> +    technical reasons, we cannot allow a slash and other characters."""
> +    return len(name) > 0 and name[0] != '.' and re.search(r'[\x00-\x20]', name) is None

I don't quite understand how the above would filter out slashes.

There are also other types of names that won't work correctly in git,
such as names starting with (double?) hyphens.

Does anyone know if it's explicitly documented anywhere which types of
names are (meant to be) allowed for git refs?
Note that you can create refs with names that don't actually work
correctly; e.g.,

 sh$ git tag -- --foo
 sh$ git rev-parse --foo
 <failure>

- Gustav

Re: [stgit PATCH] commands.{new,rename}: verify patch names

[Max Kellermann]

On 2010/10/05 14:44, Gustav Hållberg <gustav@gmail.com> wrote:
> On Tue, Oct 5, 2010 at 1:45 PM, Max Kellermann <max@duempel.org> wrote:
> > +def check_patch_name(name):
> > +    """Checks if the specified name is a valid patch name. For
> > +    technical reasons, we cannot allow a slash and other characters."""
> > +    return len(name) > 0 and name[0] != '.' and re.search(r'[\x00-\x20]', name) is None
> 
> I don't quite understand how the above would filter out slashes.

Oh damn, you're right.  I had slashes explicitly forbidden in a
previous revision of my patch, that got lost when I added my "kill all
whitespace" change.  I'll resubmit.

>  sh$ git tag -- --foo
>  sh$ git rev-parse --foo
>  <failure>

I guess this is a problem because "git-rev-parse" doesn't follow the
convention of the "--" option separator.

[stgit PATCH] commands.{new,rename}: verify patch names

[Max Kellermann]

Don't allow patches with invalid names.  For example, a patch with a
slash in the name will cause the underlying git command to fail, and
stgit doesn't handle this error condition properly.
---
 stgit/commands/new.py    |    3 +++
 stgit/commands/rename.py |    3 +++
 stgit/utils.py           |    6 ++++++
 3 files changed, 12 insertions(+), 0 deletions(-)

diff --git a/stgit/commands/new.py b/stgit/commands/new.py
index d5c5382..6bd7314 100644
--- a/stgit/commands/new.py
+++ b/stgit/commands/new.py
@@ -61,6 +61,9 @@ def func(parser, options, args):
         name = args[0]
         if stack.patches.exists(name):
             raise common.CmdException('%s: patch already exists' % name)
+
+        if not utils.check_patch_name(name):
+            raise common.CmdException('%s: invalid patch name' % name)
     else:
         parser.error('incorrect number of arguments')
 
diff --git a/stgit/commands/rename.py b/stgit/commands/rename.py
index db898cb..7c229be 100644
--- a/stgit/commands/rename.py
+++ b/stgit/commands/rename.py
@@ -51,6 +51,9 @@ def func(parser, options, args):
     else:
         parser.error('incorrect number of arguments')
 
+    if not check_patch_name(new):
+        raise CmdException('%s: invalid patch name' % new)
+
     out.start('Renaming patch "%s" to "%s"' % (old, new))
     crt_series.rename_patch(old, new)
 
diff --git a/stgit/utils.py b/stgit/utils.py
index 2955adf..a41457b 100644
--- a/stgit/utils.py
+++ b/stgit/utils.py
@@ -241,6 +241,12 @@ def make_patch_name(msg, unacceptable, default_name = 'patch'):
         patchname = default_name
     return find_patch_name(patchname, unacceptable)
 
+def check_patch_name(name):
+    """Checks if the specified name is a valid patch name. For
+    technical reasons, we cannot allow a slash and other characters."""
+    return len(name) > 0 and name[0] not in '.-' and '/' not in name and \
+           '..' not in name and re.search(r'[\x00-\x20]', name) is None
+
 # any and all functions are builtin in Python 2.5 and higher, but not
 # in 2.4.
 if not 'any' in dir(__builtins__):

Re: [stgit PATCH] commands.{new,rename}: verify patch names

[Shinya Kuribayashi]

Hi,

On 10/5/10 9:56 PM, Max Kellermann wrote:
> Don't allow patches with invalid names.  For example, a patch with a
> slash in the name will cause the underlying git command to fail, and
> stgit doesn't handle this error condition properly.
> ---
>   stgit/commands/new.py    |    3 +++
>   stgit/commands/rename.py |    3 +++
>   stgit/utils.py           |    6 ++++++
>   3 files changed, 12 insertions(+), 0 deletions(-)

It would be nice to mention about what's updated when revising
patches, even though it's even +1 line.  And we'd also like to
have a sign within Subject:, e.g., [stgit PATCH v2] will suffice.

> diff --git a/stgit/utils.py b/stgit/utils.py
> index 2955adf..a41457b 100644
> --- a/stgit/utils.py
> +++ b/stgit/utils.py
> @@ -241,6 +241,12 @@ def make_patch_name(msg, unacceptable, default_name = 'patch'):
>           patchname = default_name
>       return find_patch_name(patchname, unacceptable)
>
> +def check_patch_name(name):
> +    """Checks if the specified name is a valid patch name. For
> +    technical reasons, we cannot allow a slash and other characters."""
> +    return len(name)>  0 and name[0] not in '.-' and '/' not in name and \
> +           '..' not in name and re.search(r'[\x00-\x20]', name) is None
> +
>   # any and all functions are builtin in Python 2.5 and higher, but not
>   # in 2.4.
>   if not 'any' in dir(__builtins__):

".." is now taken care, too.  That's nice.

By the way, you're not the first person encountered this issue,
and we already have corresponding bug# at gna.org, you might be
interested in:

* https://gna.org/bugs/?10919
   sanity check patch names

* https://gna.org/bugs/?15654
   stg new when path contains slash stops stg from doing everything

I don't speak Python, so couldn't help the patch itself.
Catalin and Karl hopefully will guide you (they seem to busy
these days, or just failed to find this thread).