Patchwork [V2] mq: reject new patch name containing leading/trailing whitespace

login
register
mail settings
Submitter Yuya Nishihara
Date March 21, 2017, 2:08 a.m.
Message ID <d50f4ea6fa885c06232c.1490062103@mimosa>
Download mbox | patch
Permalink /patch/19497/
State Accepted
Headers show

Comments

Yuya Nishihara - March 21, 2017, 2:08 a.m.
# HG changeset patch
# User Yuya Nishihara <yuya@tcha.org>
# Date 1489977517 -32400
#      Mon Mar 20 11:38:37 2017 +0900
# Node ID d50f4ea6fa885c06232c18bd58f7019f2ba3767f
# Parent  44c591f634584c721778c5a77edeb04cd919ac43
mq: reject new patch name containing leading/trailing whitespace

We could create a patch of such name, but it wouldn't be processed properly
by mq as parseseries() strips leading/trailing whitespace.

The test of default message (added by b9a16ed5acec) is no longer be useful
so removed.

This issue was reported as:
https://bitbucket.org/tortoisehg/thg/issues/4693/
Ryan McElroy - March 21, 2017, 2:31 a.m.
On 3/21/17 2:08 AM, Yuya Nishihara wrote:
> # HG changeset patch
> # User Yuya Nishihara <yuya@tcha.org>
> # Date 1489977517 -32400
> #      Mon Mar 20 11:38:37 2017 +0900
> # Node ID d50f4ea6fa885c06232c18bd58f7019f2ba3767f
> # Parent  44c591f634584c721778c5a77edeb04cd919ac43
> mq: reject new patch name containing leading/trailing whitespace

This change looks good to me!

>
> We could create a patch of such name, but it wouldn't be processed properly
> by mq as parseseries() strips leading/trailing whitespace.
>
> The test of default message (added by b9a16ed5acec) is no longer be useful
> so removed.
>
> This issue was reported as:
> https://urldefense.proofpoint.com/v2/url?u=https-3A__bitbucket.org_tortoisehg_thg_issues_4693_&d=DwIGaQ&c=5VD0RTtNlTh3ycd41b3MUw&r=Jw8rundaE7TbmqBYd1txIQ&m=sADm9TGucdteBQv45e0_Iyd1sf7fj04wg_vgSIxqvcw&s=tUjOJJnFNoVJ1rhDmByv-H6ui1fbRnvQ_SA0ucpcULE&e=
>
> diff --git a/hgext/mq.py b/hgext/mq.py
> --- a/hgext/mq.py
> +++ b/hgext/mq.py
> @@ -1116,6 +1116,10 @@ class queue(object):
>           if name in self._reserved:
>               raise error.Abort(_('"%s" cannot be used as the name of a patch')
>                                % name)
> +        if name != name.strip():
> +            # whitespace is stripped by parseseries()
> +            raise error.Abort(_('patch name cannot begin or end with '
> +                                'whitespace'))
>           for prefix in ('.hg', '.mq'):
>               if name.startswith(prefix):
>                   raise error.Abort(_('patch name cannot begin with "%s"')
> diff --git a/tests/test-mq-qimport.t b/tests/test-mq-qimport.t
> --- a/tests/test-mq-qimport.t
> +++ b/tests/test-mq-qimport.t
> @@ -247,11 +247,28 @@ qimport -e --name with --force
>     this-name-is-better
>     url.diff
>   
> +import patch of bad filename
> +
> +  $ touch '../ bad.diff'
> +  $ hg qimport '../ bad.diff'
> +  abort: patch name cannot begin or end with whitespace
> +  [255]
> +  $ touch '.hg/patches/ bad.diff'
> +  $ hg qimport -e ' bad.diff'
> +  abort: patch name cannot begin or end with whitespace
> +  [255]
> +
>   qimport with bad name, should abort before reading file
>   
>     $ hg qimport non-existent-file --name .hg
>     abort: patch name cannot begin with ".hg"
>     [255]
> +  $ hg qimport non-existent-file --name ' foo'
> +  abort: patch name cannot begin or end with whitespace
> +  [255]
> +  $ hg qimport non-existent-file --name 'foo '
> +  abort: patch name cannot begin or end with whitespace
> +  [255]
>   
>   qimport http:// patch with leading slashes in url
>   
> diff --git a/tests/test-mq-qnew.t b/tests/test-mq-qnew.t
> --- a/tests/test-mq-qnew.t
> +++ b/tests/test-mq-qnew.t
> @@ -22,6 +22,8 @@
>     >     hg qnew 'foo#bar'
>     >     hg qnew 'foo:bar'
>     >     hg qnew "`echo foo; echo bar`"
> +  >     hg qnew ' foo'
> +  >     hg qnew 'foo '
>     >
>     >     hg qinit -c
>     >
> @@ -112,6 +114,8 @@ plain headers
>     abort: '#' cannot be used in the name of a patch
>     abort: ':' cannot be used in the name of a patch
>     abort: '\n' cannot be used in the name of a patch
> +  abort: patch name cannot begin or end with whitespace
> +  abort: patch name cannot begin or end with whitespace
>     % qnew with name containing slash
>     abort: path ends in directory separator: foo/ (glob)
>     abort: "foo" already exists as a directory
> @@ -180,6 +184,8 @@ hg headers
>     abort: '#' cannot be used in the name of a patch
>     abort: ':' cannot be used in the name of a patch
>     abort: '\n' cannot be used in the name of a patch
> +  abort: patch name cannot begin or end with whitespace
> +  abort: patch name cannot begin or end with whitespace
>     % qnew with name containing slash
>     abort: path ends in directory separator: foo/ (glob)
>     abort: "foo" already exists as a directory
> @@ -313,36 +319,3 @@ Test saving last-message.txt
>     > [hooks]
>     > pretxncommit.unexpectedabort =
>     > EOF
> -
> -#if unix-permissions
> -
> -Test handling default message with the patch filename with tail whitespaces
> -
> -  $ cat > $TESTTMP/editor.sh << EOF
> -  > echo "==== before editing"
> -  > cat \$1
> -  > echo "===="
> -  > echo "[mq]: patch        " > \$1
> -  > EOF
> -
> -  $ rm -f .hg/last-message.txt
> -  $ hg status
> -  $ HGEDITOR="sh $TESTTMP/editor.sh" hg qnew -e "patch "
> -  ==== before editing
> -
> -
> -  HG: Enter commit message.  Lines beginning with 'HG:' are removed.
> -  HG: Leave message empty to use default message.
> -  HG: --
> -  HG: user: test
> -  HG: branch 'default'
> -  HG: no files changed
> -  ====
> -  $ cat ".hg/patches/patch "
> -  # HG changeset patch
> -  # Parent  0000000000000000000000000000000000000000
> -
> -
> -  $ cd ..
> -
> -#endif
> diff --git a/tests/test-qrecord.t b/tests/test-qrecord.t
> --- a/tests/test-qrecord.t
> +++ b/tests/test-qrecord.t
> @@ -239,6 +239,12 @@ qrecord with bad patch name, should abor
>     $ hg qrecord .hg
>     abort: patch name cannot begin with ".hg"
>     [255]
> +  $ hg qrecord ' foo'
> +  abort: patch name cannot begin or end with whitespace
> +  [255]
> +  $ hg qrecord 'foo '
> +  abort: patch name cannot begin or end with whitespace
> +  [255]
>   
>   qrecord a.patch
>
Augie Fackler - March 21, 2017, 10:07 p.m.
On Tue, Mar 21, 2017 at 11:08:23AM +0900, Yuya Nishihara wrote:
> # HG changeset patch
> # User Yuya Nishihara <yuya@tcha.org>
> # Date 1489977517 -32400
> #      Mon Mar 20 11:38:37 2017 +0900
> # Node ID d50f4ea6fa885c06232c18bd58f7019f2ba3767f
> # Parent  44c591f634584c721778c5a77edeb04cd919ac43
> mq: reject new patch name containing leading/trailing whitespace

Queued, thanks.

>
> We could create a patch of such name, but it wouldn't be processed properly
> by mq as parseseries() strips leading/trailing whitespace.
>
> The test of default message (added by b9a16ed5acec) is no longer be useful
> so removed.
>
> This issue was reported as:
> https://bitbucket.org/tortoisehg/thg/issues/4693/
>
> diff --git a/hgext/mq.py b/hgext/mq.py
> --- a/hgext/mq.py
> +++ b/hgext/mq.py
> @@ -1116,6 +1116,10 @@ class queue(object):
>          if name in self._reserved:
>              raise error.Abort(_('"%s" cannot be used as the name of a patch')
>                               % name)
> +        if name != name.strip():
> +            # whitespace is stripped by parseseries()
> +            raise error.Abort(_('patch name cannot begin or end with '
> +                                'whitespace'))
>          for prefix in ('.hg', '.mq'):
>              if name.startswith(prefix):
>                  raise error.Abort(_('patch name cannot begin with "%s"')
> diff --git a/tests/test-mq-qimport.t b/tests/test-mq-qimport.t
> --- a/tests/test-mq-qimport.t
> +++ b/tests/test-mq-qimport.t
> @@ -247,11 +247,28 @@ qimport -e --name with --force
>    this-name-is-better
>    url.diff
>
> +import patch of bad filename
> +
> +  $ touch '../ bad.diff'
> +  $ hg qimport '../ bad.diff'
> +  abort: patch name cannot begin or end with whitespace
> +  [255]
> +  $ touch '.hg/patches/ bad.diff'
> +  $ hg qimport -e ' bad.diff'
> +  abort: patch name cannot begin or end with whitespace
> +  [255]
> +
>  qimport with bad name, should abort before reading file
>
>    $ hg qimport non-existent-file --name .hg
>    abort: patch name cannot begin with ".hg"
>    [255]
> +  $ hg qimport non-existent-file --name ' foo'
> +  abort: patch name cannot begin or end with whitespace
> +  [255]
> +  $ hg qimport non-existent-file --name 'foo '
> +  abort: patch name cannot begin or end with whitespace
> +  [255]
>
>  qimport http:// patch with leading slashes in url
>
> diff --git a/tests/test-mq-qnew.t b/tests/test-mq-qnew.t
> --- a/tests/test-mq-qnew.t
> +++ b/tests/test-mq-qnew.t
> @@ -22,6 +22,8 @@
>    >     hg qnew 'foo#bar'
>    >     hg qnew 'foo:bar'
>    >     hg qnew "`echo foo; echo bar`"
> +  >     hg qnew ' foo'
> +  >     hg qnew 'foo '
>    >
>    >     hg qinit -c
>    >
> @@ -112,6 +114,8 @@ plain headers
>    abort: '#' cannot be used in the name of a patch
>    abort: ':' cannot be used in the name of a patch
>    abort: '\n' cannot be used in the name of a patch
> +  abort: patch name cannot begin or end with whitespace
> +  abort: patch name cannot begin or end with whitespace
>    % qnew with name containing slash
>    abort: path ends in directory separator: foo/ (glob)
>    abort: "foo" already exists as a directory
> @@ -180,6 +184,8 @@ hg headers
>    abort: '#' cannot be used in the name of a patch
>    abort: ':' cannot be used in the name of a patch
>    abort: '\n' cannot be used in the name of a patch
> +  abort: patch name cannot begin or end with whitespace
> +  abort: patch name cannot begin or end with whitespace
>    % qnew with name containing slash
>    abort: path ends in directory separator: foo/ (glob)
>    abort: "foo" already exists as a directory
> @@ -313,36 +319,3 @@ Test saving last-message.txt
>    > [hooks]
>    > pretxncommit.unexpectedabort =
>    > EOF
> -
> -#if unix-permissions
> -
> -Test handling default message with the patch filename with tail whitespaces
> -
> -  $ cat > $TESTTMP/editor.sh << EOF
> -  > echo "==== before editing"
> -  > cat \$1
> -  > echo "===="
> -  > echo "[mq]: patch        " > \$1
> -  > EOF
> -
> -  $ rm -f .hg/last-message.txt
> -  $ hg status
> -  $ HGEDITOR="sh $TESTTMP/editor.sh" hg qnew -e "patch "
> -  ==== before editing
> -
> -
> -  HG: Enter commit message.  Lines beginning with 'HG:' are removed.
> -  HG: Leave message empty to use default message.
> -  HG: --
> -  HG: user: test
> -  HG: branch 'default'
> -  HG: no files changed
> -  ====
> -  $ cat ".hg/patches/patch "
> -  # HG changeset patch
> -  # Parent  0000000000000000000000000000000000000000
> -
> -
> -  $ cd ..
> -
> -#endif
> diff --git a/tests/test-qrecord.t b/tests/test-qrecord.t
> --- a/tests/test-qrecord.t
> +++ b/tests/test-qrecord.t
> @@ -239,6 +239,12 @@ qrecord with bad patch name, should abor
>    $ hg qrecord .hg
>    abort: patch name cannot begin with ".hg"
>    [255]
> +  $ hg qrecord ' foo'
> +  abort: patch name cannot begin or end with whitespace
> +  [255]
> +  $ hg qrecord 'foo '
> +  abort: patch name cannot begin or end with whitespace
> +  [255]
>
>  qrecord a.patch
>
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Patch

diff --git a/hgext/mq.py b/hgext/mq.py
--- a/hgext/mq.py
+++ b/hgext/mq.py
@@ -1116,6 +1116,10 @@  class queue(object):
         if name in self._reserved:
             raise error.Abort(_('"%s" cannot be used as the name of a patch')
                              % name)
+        if name != name.strip():
+            # whitespace is stripped by parseseries()
+            raise error.Abort(_('patch name cannot begin or end with '
+                                'whitespace'))
         for prefix in ('.hg', '.mq'):
             if name.startswith(prefix):
                 raise error.Abort(_('patch name cannot begin with "%s"')
diff --git a/tests/test-mq-qimport.t b/tests/test-mq-qimport.t
--- a/tests/test-mq-qimport.t
+++ b/tests/test-mq-qimport.t
@@ -247,11 +247,28 @@  qimport -e --name with --force
   this-name-is-better
   url.diff
 
+import patch of bad filename
+
+  $ touch '../ bad.diff'
+  $ hg qimport '../ bad.diff'
+  abort: patch name cannot begin or end with whitespace
+  [255]
+  $ touch '.hg/patches/ bad.diff'
+  $ hg qimport -e ' bad.diff'
+  abort: patch name cannot begin or end with whitespace
+  [255]
+
 qimport with bad name, should abort before reading file
 
   $ hg qimport non-existent-file --name .hg
   abort: patch name cannot begin with ".hg"
   [255]
+  $ hg qimport non-existent-file --name ' foo'
+  abort: patch name cannot begin or end with whitespace
+  [255]
+  $ hg qimport non-existent-file --name 'foo '
+  abort: patch name cannot begin or end with whitespace
+  [255]
 
 qimport http:// patch with leading slashes in url
 
diff --git a/tests/test-mq-qnew.t b/tests/test-mq-qnew.t
--- a/tests/test-mq-qnew.t
+++ b/tests/test-mq-qnew.t
@@ -22,6 +22,8 @@ 
   >     hg qnew 'foo#bar'
   >     hg qnew 'foo:bar'
   >     hg qnew "`echo foo; echo bar`"
+  >     hg qnew ' foo'
+  >     hg qnew 'foo '
   > 
   >     hg qinit -c
   > 
@@ -112,6 +114,8 @@  plain headers
   abort: '#' cannot be used in the name of a patch
   abort: ':' cannot be used in the name of a patch
   abort: '\n' cannot be used in the name of a patch
+  abort: patch name cannot begin or end with whitespace
+  abort: patch name cannot begin or end with whitespace
   % qnew with name containing slash
   abort: path ends in directory separator: foo/ (glob)
   abort: "foo" already exists as a directory
@@ -180,6 +184,8 @@  hg headers
   abort: '#' cannot be used in the name of a patch
   abort: ':' cannot be used in the name of a patch
   abort: '\n' cannot be used in the name of a patch
+  abort: patch name cannot begin or end with whitespace
+  abort: patch name cannot begin or end with whitespace
   % qnew with name containing slash
   abort: path ends in directory separator: foo/ (glob)
   abort: "foo" already exists as a directory
@@ -313,36 +319,3 @@  Test saving last-message.txt
   > [hooks]
   > pretxncommit.unexpectedabort =
   > EOF
-
-#if unix-permissions
-
-Test handling default message with the patch filename with tail whitespaces
-
-  $ cat > $TESTTMP/editor.sh << EOF
-  > echo "==== before editing"
-  > cat \$1
-  > echo "===="
-  > echo "[mq]: patch        " > \$1
-  > EOF
-
-  $ rm -f .hg/last-message.txt
-  $ hg status
-  $ HGEDITOR="sh $TESTTMP/editor.sh" hg qnew -e "patch "
-  ==== before editing
-  
-  
-  HG: Enter commit message.  Lines beginning with 'HG:' are removed.
-  HG: Leave message empty to use default message.
-  HG: --
-  HG: user: test
-  HG: branch 'default'
-  HG: no files changed
-  ====
-  $ cat ".hg/patches/patch "
-  # HG changeset patch
-  # Parent  0000000000000000000000000000000000000000
-  
-
-  $ cd ..
-
-#endif
diff --git a/tests/test-qrecord.t b/tests/test-qrecord.t
--- a/tests/test-qrecord.t
+++ b/tests/test-qrecord.t
@@ -239,6 +239,12 @@  qrecord with bad patch name, should abor
   $ hg qrecord .hg
   abort: patch name cannot begin with ".hg"
   [255]
+  $ hg qrecord ' foo'
+  abort: patch name cannot begin or end with whitespace
+  [255]
+  $ hg qrecord 'foo '
+  abort: patch name cannot begin or end with whitespace
+  [255]
 
 qrecord a.patch