bundle2: refuse empty parameter name

The bundle2 now raise value error when seeing invalid parameter names. The first
introduced rules is: no empty parameter.

The test extension is improve to properly abort when ValueError are encountered.

mercurial/bundle2.py

@@ -1,178 +1,181 b''
 # bundle2.py - generic container format to transmit arbitrary data.
 #
 # Copyright 2013 Facebook, Inc.
 #
 # This software may be used and distributed according to the terms of the
 # GNU General Public License version 2 or any later version.
 """Handling of the new bundle2 format
 
 The goal of bundle2 is to act as an atomically packet to transmit a set of
 payloads in an application agnostic way. It consist in a sequence of "parts"
 that will be handed to and processed by the application layer.
 
 
 General format architecture
 ===========================
 
 The format is architectured as follow
 
  - magic string
  - stream level parameters
  - payload parts (any number)
  - end of stream marker.
 
 The current implementation accept some stream level option but no part.
 
 Details on the Binary format
 ============================
 
 All numbers are unsigned and big endian.
 
 stream level parameters
 ------------------------
 
 Binary format is as follow
 
 :params size: (16 bits integer)
 
   The total number of Bytes used by the parameters
 
 :params value: arbitrary number of Bytes
 
   A blob of `params size` containing the serialized version of all stream level
   parameters.
 
   The blob contains a space separated list of parameters. parameter with value
   are stored in the form `<name>=<value>`. Both name and value are urlquoted.
 
+  Empty name are obviously forbidden.
+
   Stream parameters use a simple textual format for two main reasons:
 
   - Stream level parameters should remains simple and we want to discourage any
     crazy usage.
   - Textual data allow easy human inspection of a the bundle2 header in case of
     troubles.
 
   Any Applicative level options MUST go into a bundle2 part instead.
 
-
 Payload part
 ------------------------
 
 Binary format is as follow
 
 :header size: (16 bits inter)
 
   The total number of Bytes used by the part headers. When the header is empty
   (size = 0) this is interpreted as the end of stream marker.
 
   Currently forced to 0 in the current state of the implementation
 """
 
 import util
 import struct
 import urllib
 
 import changegroup
 from i18n import _
 
 _pack = struct.pack
 _unpack = struct.unpack
 
 _magicstring = 'HG20'
 
 _fstreamparamsize = '>H'
 
 class bundle20(object):
     """represent an outgoing bundle2 container
 
     Use the `addparam` method to add stream level parameter. Then call
     `getchunks` to retrieve all the binary chunks of datathat compose the
     bundle2 container.
 
     This object does not support payload part yet."""
 
     def __init__(self):
         self._params = []
         self._parts = []
 
     def addparam(self, name, value=None):
         """add a stream level parameter"""
+        if not name:
+            raise ValueError('empty parameter name')
         self._params.append((name, value))
 
     def getchunks(self):
         yield _magicstring
         param = self._paramchunk()
         yield _pack(_fstreamparamsize, len(param))
         if param:
             yield param
 
         # no support for parts
         # to be obviously fixed soon.
         assert not self._parts
         yield '\0\0'
 
     def _paramchunk(self):
         """return a encoded version of all stream parameters"""
         blocks = []
         for par, value in self._params:
             par = urllib.quote(par)
             if value is not None:
                 value = urllib.quote(value)
                 par = '%s=%s' % (par, value)
             blocks.append(par)
         return ' '.join(blocks)
 
 class unbundle20(object):
     """interpret a bundle2 stream
 
     (this will eventually yield parts)"""
 
     def __init__(self, fp):
         self._fp = fp
         header = self._readexact(4)
         magic, version = header[0:2], header[2:4]
         if magic != 'HG':
             raise util.Abort(_('not a Mercurial bundle'))
         if version != '20':
             raise util.Abort(_('unknown bundle version %s') % version)
 
     def _unpack(self, format):
         """unpack this struct format from the stream"""
         data = self._readexact(struct.calcsize(format))
         return _unpack(format, data)
 
     def _readexact(self, size):
         """read exactly <size> bytes from the stream"""
         return changegroup.readexactly(self._fp, size)
 
     @util.propertycache
     def params(self):
         """dictionnary of stream level parameters"""
         params = {}
         paramssize = self._unpack(_fstreamparamsize)[0]
         if paramssize:
             for p in self._readexact(paramssize).split(' '):
                 p = p.split('=', 1)
                 p = [urllib.unquote(i) for i in p]
                 if len(p) < 2:
                     p.append(None)
                 params[p[0]] = p[1]
         return params
 
     def __iter__(self):
         """yield all parts contained in the stream"""
         # make sure param have been loaded
         self.params
         part = self._readpart()
         while part is not None:
             yield part
             part = self._readpart()
 
     def _readpart(self):
         """return None when an end of stream markers is reach"""
         headersize = self._readexact(2)
         assert headersize == '\0\0'
         return None
 
 
 

tests/test-bundle2.t

@@ -1,151 +1,164 b''
 
 Create an extension to test bundle2 API
 
   $ cat > bundle2.py << EOF
   > """A small extension to test bundle2 implementation
   > 
   > Current bundle2 implementation is far too limited to be used in any core
   > code. We still need to be able to test it while it grow up.
   > """
   > 
   > import sys
   > from mercurial import cmdutil
+  > from mercurial import util
   > from mercurial import bundle2
   > cmdtable = {}
   > command = cmdutil.command(cmdtable)
   > 
   > @command('bundle2',
   >          [('', 'param', [], 'stream level parameter'),],
   >          '')
   > def cmdbundle2(ui, repo, **opts):
   >     """write a bundle2 container on standard ouput"""
   >     bundler = bundle2.bundle20()
   >     for p in opts['param']:
   >         p = p.split('=', 1)
+  >         try:
   >         bundler.addparam(*p)
+  >         except ValueError, exc:
+  >             raise util.Abort('%s' % exc)
   > 
   >     for chunk in bundler.getchunks():
   >         ui.write(chunk)
   > 
   > @command('unbundle2', [], '')
   > def cmdunbundle2(ui, repo):
   >     """read a bundle2 container from standard input"""
   >     unbundler = bundle2.unbundle20(sys.stdin)
   >     ui.write('options count: %i\n' % len(unbundler.params))
   >     for key in sorted(unbundler.params):
   >         ui.write('- %s\n' % key)
   >         value = unbundler.params[key]
   >         if value is not None:
   >             ui.write('    %s\n' % value)
   >     parts = list(unbundler)
   >     ui.write('parts count:   %i\n' % len(parts))
   > EOF
   $ cat >> $HGRCPATH << EOF
   > [extensions]
   > bundle2=$TESTTMP/bundle2.py
   > EOF
 
 The extension requires a repo (currently unused)
 
   $ hg init main
   $ cd main
   $ touch a
   $ hg add a
   $ hg commit -m 'a'
 
 
 Empty bundle
 =================
 
 - no option
 - no parts
 
 Test bundling
 
   $ hg bundle2
   HG20\x00\x00\x00\x00 (no-eol) (esc)
 
 Test unbundling
 
   $ hg bundle2 | hg unbundle2
   options count: 0
   parts count:   0
 
 Test old style bundle are detected and refused
 
   $ hg bundle --all ../bundle.hg
   1 changesets found
   $ hg unbundle2 < ../bundle.hg
   abort: unknown bundle version 10
   [255]
 
 Test parameters
 =================
 
 - some options
 - no parts
 
 advisory parameters, no value
 -------------------------------
 
 Simplest possible parameters form
 
 Test generation simple option
 
   $ hg bundle2 --param 'caution'
   HG20\x00\x07caution\x00\x00 (no-eol) (esc)
 
 Test unbundling
 
   $ hg bundle2 --param 'caution' | hg unbundle2
   options count: 1
   - caution
   parts count:   0
 
 Test generation multiple option
 
   $ hg bundle2 --param 'caution' --param 'meal'
   HG20\x00\x0ccaution meal\x00\x00 (no-eol) (esc)
 
 Test unbundling
 
   $ hg bundle2 --param 'caution' --param 'meal' | hg unbundle2
   options count: 2
   - caution
   - meal
   parts count:   0
 
 advisory parameters, with value
 -------------------------------
 
 Test generation
 
   $ hg bundle2 --param 'caution' --param 'meal=vegan' --param 'elephants'
   HG20\x00\x1ccaution meal=vegan elephants\x00\x00 (no-eol) (esc)
 
 Test unbundling
 
   $ hg bundle2 --param 'caution' --param 'meal=vegan' --param 'elephants' | hg unbundle2
   options count: 3
   - caution
   - elephants
   - meal
       vegan
   parts count:   0
 
 parameter with special char in value
 ---------------------------------------------------
 
 Test generation
 
   $ hg bundle2 --param 'e|! 7/=babar%#==tutu' --param simple
   HG20\x00)e%7C%21%207/=babar%25%23%3D%3Dtutu simple\x00\x00 (no-eol) (esc)
 
 Test unbundling
 
   $ hg bundle2 --param 'e|! 7/=babar%#==tutu' --param simple | hg unbundle2
   options count: 2
   - e|! 7/
       babar%#==tutu
   - simple
   parts count:   0
+
+Test buggy input
+---------------------------------------------------
+
+empty parameter name
+
+  $ hg bundle2 --param '' --quiet
+  abort: empty parameter name
+  [255]