D4466: wireprotoframing: use our CBOR module

[indygreg (Gregory Szorc)]

indygreg created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  Tests changed because our CBOR encoder appears to sort map keys
  differently from the vendored CBOR package. The CBOR specification
  does define canonical sorting rules for keys based on the
  byte values. I'm guessing our implementation doesn't follow them.
  
  But our encoder doesn't guarantee that it conforms with the canonical
  specification. Right now, we just care that output is deterministic.
  And our encoder does guarantee that.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D4466

AFFECTED FILES
  mercurial/wireprotoframing.py
  tests/test-http-api-httpv2.t
  tests/test-wireproto-command-pushkey.t

CHANGE DETAILS

diff --git a/tests/test-wireproto-command-pushkey.t b/tests/test-wireproto-command-pushkey.t
--- a/tests/test-wireproto-command-pushkey.t
+++ b/tests/test-wireproto-command-pushkey.t
@@ -38,7 +38,7 @@
   s>     host: $LOCALIP:$HGPORT\r\n (glob)
   s>     user-agent: Mercurial debugwireproto\r\n
   s>     \r\n
-  s>     a\x00\x00\x01\x00\x01\x01\x11\xa2Dargs\xa4CkeyA at CnewX(426bada5c67598ca65036d57d9e4b64b0c1ce7a0Cold at InamespaceIbookmarksDnameGpushkey
+  s>     a\x00\x00\x01\x00\x01\x01\x11\xa2Dargs\xa4CkeyA at InamespaceIbookmarksCnewX(426bada5c67598ca65036d57d9e4b64b0c1ce7a0Cold at DnameGpushkey
   s> makefile('rb', None)
   s>     HTTP/1.1 200 OK\r\n
   s>     Server: testing stub value\r\n
diff --git a/tests/test-http-api-httpv2.t b/tests/test-http-api-httpv2.t
--- a/tests/test-http-api-httpv2.t
+++ b/tests/test-http-api-httpv2.t
@@ -406,15 +406,15 @@
   s>     content-length: 47\r\n
   s>     host: $LOCALIP:$HGPORT\r\n (glob)
   s>     \r\n
-  s>     \'\x00\x00\x01\x00\x01\x01\x11\xa2Dargs\xa2CfooDval1Dbar1CvalDnameHcommand1
+  s>     \'\x00\x00\x01\x00\x01\x01\x11\xa2Dargs\xa2Dbar1CvalCfooDval1DnameHcommand1
   s> makefile('rb', None)
   s>     HTTP/1.1 200 OK\r\n
   s>     Server: testing stub value\r\n
   s>     Date: $HTTP_DATE$\r\n
   s>     Content-Type: text/plain\r\n
   s>     Content-Length: 205\r\n
   s>     \r\n
-  s>     received: 1 1 1 \xa2Dargs\xa2CfooDval1Dbar1CvalDnameHcommand1\n
+  s>     received: 1 1 1 \xa2Dargs\xa2Dbar1CvalCfooDval1DnameHcommand1\n
   s>     ["runcommand", {"args": {"bar1": "val", "foo": "val1"}, "command": "command1", "data": null, "requestid": 1}]\n
   s>     received: <no frame>\n
   s>     {"action": "noop"}
diff --git a/mercurial/wireprotoframing.py b/mercurial/wireprotoframing.py
--- a/mercurial/wireprotoframing.py
+++ b/mercurial/wireprotoframing.py
@@ -17,14 +17,14 @@
 from .i18n import _
 from .thirdparty import (
     attr,
-    cbor,
 )
 from . import (
     encoding,
     error,
     util,
 )
 from .utils import (
+    cborutil,
     stringutil,
 )
 
@@ -217,8 +217,8 @@
             finalflags |= int(flag)
 
     if payload.startswith(b'cbor:'):
-        payload = cbor.dumps(stringutil.evalpythonliteral(payload[5:]),
-                             canonical=True)
+        payload = b''.join(cborutil.streamencode(
+            stringutil.evalpythonliteral(payload[5:])))
 
     else:
         payload = stringutil.unescapestr(payload)
@@ -289,7 +289,7 @@
     if args:
         data[b'args'] = args
 
-    data = cbor.dumps(data, canonical=True)
+    data = b''.join(cborutil.streamencode(data))
 
     offset = 0
 
@@ -347,7 +347,7 @@
     Returns a generator of bytearrays.
     """
     # Automatically send the overall CBOR response map.
-    overall = cbor.dumps({b'status': b'ok'}, canonical=True)
+    overall = b''.join(cborutil.streamencode({b'status': b'ok'}))
     if len(overall) > maxframesize:
         raise error.ProgrammingError('not yet implemented')
 
@@ -388,7 +388,7 @@
 
 def createbytesresponseframesfromgen(stream, requestid, gen,
                                      maxframesize=DEFAULT_MAX_FRAME_SIZE):
-    overall = cbor.dumps({b'status': b'ok'}, canonical=True)
+    overall = b''.join(cborutil.streamencode({b'status': b'ok'}))
 
     yield stream.makeframe(requestid=requestid,
                            typeid=FRAME_TYPE_COMMAND_RESPONSE,
@@ -431,7 +431,7 @@
     if args:
         m[b'error'][b'args'] = args
 
-    overall = cbor.dumps(m, canonical=True)
+    overall = b''.join(cborutil.streamencode(m))
 
     yield stream.makeframe(requestid=requestid,
                            typeid=FRAME_TYPE_COMMAND_RESPONSE,
@@ -442,10 +442,10 @@
     # TODO properly handle frame size limits.
     assert len(msg) <= DEFAULT_MAX_FRAME_SIZE
 
-    payload = cbor.dumps({
+    payload = b''.join(cborutil.streamencode({
         b'type': errtype,
         b'message': [{b'msg': msg}],
-    }, canonical=True)
+    }))
 
     yield stream.makeframe(requestid=requestid,
                            typeid=FRAME_TYPE_ERROR_RESPONSE,
@@ -495,7 +495,7 @@
 
         atomdicts.append(atom)
 
-    payload = cbor.dumps(atomdicts, canonical=True)
+    payload = b''.join(cborutil.streamencode(atomdicts))
 
     if len(payload) > maxframesize:
         raise ValueError('cannot encode data in a single frame')
@@ -786,7 +786,7 @@
 
         # Decode the payloads as CBOR.
         entry['payload'].seek(0)
-        request = cbor.load(entry['payload'])
+        request = cborutil.decodeall(entry['payload'].getvalue())[0]
 
         if b'name' not in request:
             self._state = 'errored'
@@ -1160,7 +1160,7 @@
         del self._activerequests[request.requestid]
 
         # The payload should be a CBOR map.
-        m = cbor.loads(frame.payload)
+        m = cborutil.decodeall(frame.payload)[0]
 
         return 'error', {
             'request': request,



To: indygreg, #hg-reviewers
Cc: mercurial-devel