Patchwork [1,of,2,STABLE] exchange: reject new compression engines for v1 bundles (issue5506)

login
register
mail settings
Submitter Gregory Szorc
Date March 16, 2017, 7:33 p.m.
Message ID <8fed20619acd7a434640.1489692813@ubuntu-vm-main>
Download mbox | patch
Permalink /patch/19399/
State Accepted
Headers show

Comments

Gregory Szorc - March 16, 2017, 7:33 p.m.
# HG changeset patch
# User Gregory Szorc <gregory.szorc@gmail.com>
# Date 1489692236 25200
#      Thu Mar 16 12:23:56 2017 -0700
# Branch stable
# Node ID 8fed20619acd7a4346408e5819a1c91c52a39ed2
# Parent  86cd1f2cfff5d5f5ebda227b147ad71a59058323
exchange: reject new compression engines for v1 bundles (issue5506)

Version 1 bundles only support a fixed set of compression engines.
Before this change, we would accept any compression engine for v1
bundles, even those that may not work on v1. This could lead to
an error.

We define a fixed set of compression engines known to work with v1
bundles and we add checking to ensure a newer engine (like zstd)
won't work with v1 bundles.

I also took the liberty of adding test coverage for unknown compression
names because I noticed we didn't have coverage of it before.
Katsunori FUJIWARA - March 17, 2017, 11:26 a.m.
At Thu, 16 Mar 2017 12:33:33 -0700,
Gregory Szorc wrote:
> 
> # HG changeset patch
> # User Gregory Szorc <gregory.szorc@gmail.com>
> # Date 1489692236 25200
> #      Thu Mar 16 12:23:56 2017 -0700
> # Branch stable
> # Node ID 8fed20619acd7a4346408e5819a1c91c52a39ed2
> # Parent  86cd1f2cfff5d5f5ebda227b147ad71a59058323
> exchange: reject new compression engines for v1 bundles (issue5506)

LGTM. Thank you for quick fixing !

> Version 1 bundles only support a fixed set of compression engines.
> Before this change, we would accept any compression engine for v1
> bundles, even those that may not work on v1. This could lead to
> an error.
> 
> We define a fixed set of compression engines known to work with v1
> bundles and we add checking to ensure a newer engine (like zstd)
> won't work with v1 bundles.
> 
> I also took the liberty of adding test coverage for unknown compression
> names because I noticed we didn't have coverage of it before.
> 
> diff --git a/mercurial/exchange.py b/mercurial/exchange.py
> --- a/mercurial/exchange.py
> +++ b/mercurial/exchange.py
> @@ -44,6 +44,9 @@ urlreq = util.urlreq
>                           'bundle2': '02', #legacy
>                          }
>  
> +# Compression engines allowed in version 1. THIS SHOULD NEVER CHANGE.
> +_bundlespecv1compengines = set(['gzip', 'bzip2', 'none'])
> +
>  def parsebundlespec(repo, spec, strict=True, externalnames=False):
>      """Parse a bundle string specification into parts.
>  
> @@ -139,6 +142,12 @@ def parsebundlespec(repo, spec, strict=T
>              raise error.UnsupportedBundleSpecification(
>                      _('%s is not a recognized bundle specification') % spec)
>  
> +    # Bundle version 1 only supports a known set of compression engines.
> +    if version == 'v1' and compression not in _bundlespecv1compengines:
> +        raise error.UnsupportedBundleSpecification(
> +            _('compression engine %s is not supported on v1 bundles') %
> +            compression)
> +
>      # The specification for packed1 can optionally declare the data formats
>      # required to apply it. If we see this metadata, compare against what the
>      # repo supports and error if the bundle isn't compatible.
> diff --git a/tests/test-bundle-type.t b/tests/test-bundle-type.t
> --- a/tests/test-bundle-type.t
> +++ b/tests/test-bundle-type.t
> @@ -33,6 +33,23 @@ bundle w/o type option
>    summary:     a
>    $ cd ..
>  
> +Unknown compression type is rejected
> +
> +  $ hg init t3
> +  $ cd t3
> +  $ hg -q pull ../b1
> +  $ hg bundle -a -t unknown out.hg
> +  abort: unknown is not a recognized bundle specification
> +  (see 'hg help bundle' for supported values for --type)
> +  [255]
> +
> +  $ hg bundle -a -t unknown-v2 out.hg
> +  abort: unknown compression is not supported
> +  (see 'hg help bundle' for supported values for --type)
> +  [255]
> +
> +  $ cd ..
> +
>  test bundle types
>  
>    $ testbundle() {
> @@ -164,6 +181,23 @@ Compression level can be adjusted for bu
>        c35a0f9217e65d1fdb90c936ffa7dbe679f83ddf
>    zstd-v2
>    
> +
> +Explicit request for zstd on non-generaldelta repos
> +
> +  $ hg --config format.usegeneraldelta=false init nogd
> +  $ hg -q -R nogd pull t1
> +  $ hg -R nogd bundle -a -t zstd nogd-zstd
> +  abort: compression engine zstd is not supported on v1 bundles
> +  (see 'hg help bundle' for supported values for --type)
> +  [255]
> +
> +zstd-v1 always fails
> +
> +  $ hg -R tzstd bundle -a -t zstd-v1 zstd-v1
> +  abort: compression engine zstd is not supported on v1 bundles
> +  (see 'hg help bundle' for supported values for --type)
> +  [255]
> +
>  #else
>  
>  zstd is a valid engine but isn't available
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Yuya Nishihara - March 18, 2017, 3:02 a.m.
On Fri, 17 Mar 2017 20:26:39 +0900, FUJIWARA Katsunori wrote:
> At Thu, 16 Mar 2017 12:33:33 -0700,
> Gregory Szorc wrote:
> > 
> > # HG changeset patch
> > # User Gregory Szorc <gregory.szorc@gmail.com>
> > # Date 1489692236 25200
> > #      Thu Mar 16 12:23:56 2017 -0700
> > # Branch stable
> > # Node ID 8fed20619acd7a4346408e5819a1c91c52a39ed2
> > # Parent  86cd1f2cfff5d5f5ebda227b147ad71a59058323
> > exchange: reject new compression engines for v1 bundles (issue5506)
> 
> LGTM. Thank you for quick fixing !

Yeah, queued these for stable, thanks.

Patch

diff --git a/mercurial/exchange.py b/mercurial/exchange.py
--- a/mercurial/exchange.py
+++ b/mercurial/exchange.py
@@ -44,6 +44,9 @@  urlreq = util.urlreq
                          'bundle2': '02', #legacy
                         }
 
+# Compression engines allowed in version 1. THIS SHOULD NEVER CHANGE.
+_bundlespecv1compengines = set(['gzip', 'bzip2', 'none'])
+
 def parsebundlespec(repo, spec, strict=True, externalnames=False):
     """Parse a bundle string specification into parts.
 
@@ -139,6 +142,12 @@  def parsebundlespec(repo, spec, strict=T
             raise error.UnsupportedBundleSpecification(
                     _('%s is not a recognized bundle specification') % spec)
 
+    # Bundle version 1 only supports a known set of compression engines.
+    if version == 'v1' and compression not in _bundlespecv1compengines:
+        raise error.UnsupportedBundleSpecification(
+            _('compression engine %s is not supported on v1 bundles') %
+            compression)
+
     # The specification for packed1 can optionally declare the data formats
     # required to apply it. If we see this metadata, compare against what the
     # repo supports and error if the bundle isn't compatible.
diff --git a/tests/test-bundle-type.t b/tests/test-bundle-type.t
--- a/tests/test-bundle-type.t
+++ b/tests/test-bundle-type.t
@@ -33,6 +33,23 @@  bundle w/o type option
   summary:     a
   $ cd ..
 
+Unknown compression type is rejected
+
+  $ hg init t3
+  $ cd t3
+  $ hg -q pull ../b1
+  $ hg bundle -a -t unknown out.hg
+  abort: unknown is not a recognized bundle specification
+  (see 'hg help bundle' for supported values for --type)
+  [255]
+
+  $ hg bundle -a -t unknown-v2 out.hg
+  abort: unknown compression is not supported
+  (see 'hg help bundle' for supported values for --type)
+  [255]
+
+  $ cd ..
+
 test bundle types
 
   $ testbundle() {
@@ -164,6 +181,23 @@  Compression level can be adjusted for bu
       c35a0f9217e65d1fdb90c936ffa7dbe679f83ddf
   zstd-v2
   
+
+Explicit request for zstd on non-generaldelta repos
+
+  $ hg --config format.usegeneraldelta=false init nogd
+  $ hg -q -R nogd pull t1
+  $ hg -R nogd bundle -a -t zstd nogd-zstd
+  abort: compression engine zstd is not supported on v1 bundles
+  (see 'hg help bundle' for supported values for --type)
+  [255]
+
+zstd-v1 always fails
+
+  $ hg -R tzstd bundle -a -t zstd-v1 zstd-v1
+  abort: compression engine zstd is not supported on v1 bundles
+  (see 'hg help bundle' for supported values for --type)
+  [255]
+
 #else
 
 zstd is a valid engine but isn't available