Patchwork [3,of,3] bundle2: handle long error params on the unbundling side

login
register
mail settings
Submitter Siddharth Agarwal
Date April 4, 2017, 11:43 p.m.
Message ID <0f7eea5bae914c2ec36f.1491349401@devvm002.prn1.facebook.com>
Download mbox | patch
Permalink /patch/19964/
State Superseded
Headers show

Comments

Siddharth Agarwal - April 4, 2017, 11:43 p.m.
# HG changeset patch
# User Siddharth Agarwal <sid0@fb.com>
# Date 1491349317 25200
#      Tue Apr 04 16:41:57 2017 -0700
# Node ID 0f7eea5bae914c2ec36f004e214a336054287727
# Parent  f31d776ec35686693b3521c07a61ade94e567b9a
bundle2: handle long error params on the unbundling side

As the tests establish, the unbundling side can now present full parameters.

We retain tests to simulate an old client, and add tests to simulate an old
server talking to a new client.

Patch

diff --git a/mercurial/bundle2.py b/mercurial/bundle2.py
--- a/mercurial/bundle2.py
+++ b/mercurial/bundle2.py
@@ -1556,11 +1556,37 @@  def handlereplycaps(op, inpart):
 class AbortFromPart(error.Abort):
     """Sub-class of Abort that denotes an error from a bundle2 part."""
 
+def _errorparams(inpart):
+    """Read error parameters from the payload if so specified.
+
+    This should only be used for errors generated via bundleerrorpart."""
+    params = inpart.params
+    if params.get('longparams') != 'payload':
+        return params
+    # avoid modifying the params dict in inpart
+    params = params.copy()
+    # don't use inpart._unpack/_readexact because they read the underlying
+    # stream (with payload chunk sizes etc) -- instead read from the
+    # higher-level stream
+    lensize = struct.calcsize(_ferrorlongparamsize)
+    def readsize():
+        return struct.unpack(_ferrorlongparamsize,
+                             changegroup.readexactly(inpart, lensize))[0]
+
+    numparams = readsize()
+    for _param in xrange(numparams):
+        namesize = readsize()
+        name = changegroup.readexactly(inpart, namesize)
+        valuesize = readsize()
+        value = changegroup.readexactly(inpart, valuesize)
+        params[name] = value
+    return params
+
 @parthandler('error:abort', ('message', 'hint'))
 def handleerrorabort(op, inpart):
     """Used to transmit abort error over the wire"""
-    raise AbortFromPart(inpart.params['message'],
-                        hint=inpart.params.get('hint'))
+    params = _errorparams(inpart)
+    raise AbortFromPart(params['message'], hint=params.get('hint'))
 
 @parthandler('error:pushkey', ('namespace', 'key', 'new', 'old', 'ret',
                                'in-reply-to'))
@@ -1576,11 +1602,12 @@  def handleerrorpushkey(op, inpart):
 @parthandler('error:unsupportedcontent', ('parttype', 'params'))
 def handleerrorunsupportedcontent(op, inpart):
     """Used to transmit unknown content error over the wire"""
+    params = _errorparams(inpart)
     kwargs = {}
-    parttype = inpart.params.get('parttype')
+    parttype = params.get('parttype')
     if parttype is not None:
         kwargs['parttype'] = parttype
-    params = inpart.params.get('params')
+    params = params.get('params')
     if params is not None:
         kwargs['params'] = params.split('\0')
 
@@ -1589,7 +1616,8 @@  def handleerrorunsupportedcontent(op, in
 @parthandler('error:pushraced', ('message',))
 def handleerrorpushraced(op, inpart):
     """Used to transmit push race error over the wire"""
-    raise error.ResponseError(_('push failed:'), inpart.params['message'])
+    params = _errorparams(inpart)
+    raise error.ResponseError(_('push failed:'), params['message'])
 
 @parthandler('listkeys', ('namespace',))
 def handlelistkeys(op, inpart):
diff --git a/tests/test-bundle2-exchange.t b/tests/test-bundle2-exchange.t
--- a/tests/test-bundle2-exchange.t
+++ b/tests/test-bundle2-exchange.t
@@ -464,6 +464,10 @@  Setting up
   >         bundler.newpart('test:abort')
   >     if reason == 'abort-long':
   >         bundler.newpart('test:abort-long')
+  >     if reason == 'abort-legacy':
+  >         # Use bundler.newpart rather than bundler.newerrorpart to create this part.
+  >         # This simulates a Mercurial server < 4.2.
+  >         bundler.newpart('error:abort', [('message', 'Abandon ship too!')])
   >     if reason == 'unknown':
   >         bundler.newpart('test:unknown')
   >     if reason == 'race':
@@ -480,9 +484,16 @@  Setting up
   >     # across payload chunks
   >     raise error.Abort('a' * 8192, hint="don't panic")
   > 
+  > def overrideerrorparams(orig, inpart):
+  >     # simulate Mercurial < 4.2 if this config is set
+  >     if inpart.ui.configbool('failpush', 'legacyerrorparams'):
+  >         return inpart.params
+  >     return orig(inpart)
+  > 
   > def uisetup(ui):
   >     exchange.b2partsgenmapping['failpart'] = _pushbundle2failpart
   >     exchange.b2partsgenorder.insert(0, 'failpart')
+  >     extensions.wrapfunction(bundle2, '_errorparams', overrideerrorparams)
   > 
   > EOF
 
@@ -543,6 +554,22 @@  Doing the actual push: Abort error (mess
   $ cat << EOF >> $HGRCPATH
   > [failpush]
   > reason = abort-long
+  > legacyerrorparams = false
+  > EOF
+
+  $ hg -R main push ssh://user@dummy/other -r e7ec4e813ba6
+  pushing to ssh://user@dummy/other
+  searching for changes
+  remote: (a){8192} (re)
+  remote: (don't panic)
+  abort: push failed on remote
+  [255]
+
+Abort error (simulate client Mercurial < 4.2 -- truncated message)
+
+  $ cat << EOF >> $HGRCPATH
+  > [failpush]
+  > legacyerrorparams = true
   > EOF
 
   $ hg -R main push ssh://user@dummy/other -r e7ec4e813ba6
@@ -553,6 +580,25 @@  Doing the actual push: Abort error (mess
   abort: push failed on remote
   [255]
 
+  $ cat << EOF >> $HGRCPATH
+  > [failpush]
+  > legacyerrorparams = false
+  > EOF
+
+Abort error (simulate server Mercurial < 4.2)
+
+  $ cat << EOF >> $HGRCPATH
+  > [failpush]
+  > reason = abort-legacy
+  > EOF
+
+  $ hg -R main push ssh://user@dummy/other -r e7ec4e813ba6
+  pushing to ssh://user@dummy/other
+  searching for changes
+  remote: Abandon ship too!
+  abort: push failed on remote
+  [255]
+
 Doing the actual push: unknown mandatory parts
 
   $ cat << EOF >> $HGRCPATH