D3067: wireproto: convert human output frames to CBOR
indygreg (Gregory Szorc)
phabricator at mercurial-scm.org
Wed Apr 4 02:00:23 UTC 2018
indygreg created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.
REVISION SUMMARY
This is easier than rolling our own encoding format.
As a bonus, some of our artificial limits around lengths of
things went away because we are no longer using fixed length
fields to hold sizes.
REPOSITORY
rHG Mercurial
REVISION DETAIL
https://phab.mercurial-scm.org/D3067
AFFECTED FILES
mercurial/help/internals/wireprotocol.txt
mercurial/wireprotoframing.py
tests/test-wireproto-serverreactor.py
CHANGE DETAILS
diff --git a/tests/test-wireproto-serverreactor.py b/tests/test-wireproto-serverreactor.py
--- a/tests/test-wireproto-serverreactor.py
+++ b/tests/test-wireproto-serverreactor.py
@@ -136,22 +136,6 @@
ffs(b'1 1 0 command-data eos %s' % data.getvalue()),
])
- def testtextoutputexcessiveargs(self):
- """At most 255 formatting arguments are allowed."""
- with self.assertRaisesRegexp(ValueError,
- 'cannot use more than 255 formatting'):
- args = [b'x' for i in range(256)]
- list(framing.createtextoutputframe(None, 1,
- [(b'bleh', args, [])]))
-
- def testtextoutputexcessivelabels(self):
- """At most 255 labels are allowed."""
- with self.assertRaisesRegexp(ValueError,
- 'cannot use more than 255 labels'):
- labels = [b'l' for i in range(256)]
- list(framing.createtextoutputframe(None, 1,
- [(b'bleh', [], labels)]))
-
def testtextoutputformattingstringtype(self):
"""Formatting string must be bytes."""
with self.assertRaisesRegexp(ValueError, 'must use bytes formatting '):
@@ -168,31 +152,14 @@
list(framing.createtextoutputframe(None, 1, [
(b'foo', [], [b'foo'.decode('ascii')])]))
- def testtextoutputtoolongformatstring(self):
- with self.assertRaisesRegexp(ValueError,
- 'formatting string cannot be longer than'):
- list(framing.createtextoutputframe(None, 1, [
- (b'x' * 65536, [], [])]))
-
- def testtextoutputtoolongargumentstring(self):
- with self.assertRaisesRegexp(ValueError,
- 'argument string cannot be longer than'):
- list(framing.createtextoutputframe(None, 1, [
- (b'bleh', [b'x' * 65536], [])]))
-
- def testtextoutputtoolonglabelstring(self):
- with self.assertRaisesRegexp(ValueError,
- 'label string cannot be longer than'):
- list(framing.createtextoutputframe(None, 1, [
- (b'bleh', [], [b'x' * 65536])]))
-
def testtextoutput1simpleatom(self):
stream = framing.stream(1)
val = list(framing.createtextoutputframe(stream, 1, [
(b'foo', [], [])]))
self.assertEqual(val, [
- ffs(br'1 1 stream-begin text-output 0 \x03\x00\x00\x00foo'),
+ ffs(b'1 1 stream-begin text-output 0 '
+ b"cbor:[{b'msg': b'foo'}]"),
])
def testtextoutput2simpleatoms(self):
@@ -203,8 +170,8 @@
]))
self.assertEqual(val, [
- ffs(br'1 1 stream-begin text-output 0 '
- br'\x03\x00\x00\x00foo\x03\x00\x00\x00bar'),
+ ffs(b'1 1 stream-begin text-output 0 '
+ b"cbor:[{b'msg': b'foo'}, {b'msg': b'bar'}]")
])
def testtextoutput1arg(self):
@@ -214,8 +181,8 @@
]))
self.assertEqual(val, [
- ffs(br'1 1 stream-begin text-output 0 '
- br'\x06\x00\x00\x01\x04\x00foo %sval1'),
+ ffs(b'1 1 stream-begin text-output 0 '
+ b"cbor:[{b'msg': b'foo %s', b'args': [b'val1']}]")
])
def testtextoutput2arg(self):
@@ -225,8 +192,8 @@
]))
self.assertEqual(val, [
- ffs(br'1 1 stream-begin text-output 0 '
- br'\x09\x00\x00\x02\x03\x00\x05\x00foo %s %svalvalue'),
+ ffs(b'1 1 stream-begin text-output 0 '
+ b"cbor:[{b'msg': b'foo %s %s', b'args': [b'val', b'value']}]")
])
def testtextoutput1label(self):
@@ -236,8 +203,8 @@
]))
self.assertEqual(val, [
- ffs(br'1 1 stream-begin text-output 0 '
- br'\x03\x00\x01\x00\x05foolabel'),
+ ffs(b'1 1 stream-begin text-output 0 '
+ b"cbor:[{b'msg': b'foo', b'labels': [b'label']}]")
])
def testargandlabel(self):
@@ -247,8 +214,9 @@
]))
self.assertEqual(val, [
- ffs(br'1 1 stream-begin text-output 0 '
- br'\x06\x00\x01\x01\x05\x03\x00foo %slabelarg'),
+ ffs(b'1 1 stream-begin text-output 0 '
+ b"cbor:[{b'msg': b'foo %s', b'args': [b'arg'], "
+ b"b'labels': [b'label']}]")
])
class ServerReactorTests(unittest.TestCase):
diff --git a/mercurial/wireprotoframing.py b/mercurial/wireprotoframing.py
--- a/mercurial/wireprotoframing.py
+++ b/mercurial/wireprotoframing.py
@@ -395,7 +395,8 @@
flags=flags,
payload=msg)
-def createtextoutputframe(stream, requestid, atoms):
+def createtextoutputframe(stream, requestid, atoms,
+ maxframesize=DEFAULT_MAX_FRAME_SIZE):
"""Create a text output frame to render text to people.
``atoms`` is a 3-tuple of (formatting string, args, labels).
@@ -405,15 +406,9 @@
formatters to be applied at rendering time. In terms of the ``ui``
class, each atom corresponds to a ``ui.write()``.
"""
- bytesleft = DEFAULT_MAX_FRAME_SIZE
- atomchunks = []
+ atomdicts = []
for (formatting, args, labels) in atoms:
- if len(args) > 255:
- raise ValueError('cannot use more than 255 formatting arguments')
- if len(labels) > 255:
- raise ValueError('cannot use more than 255 labels')
-
# TODO look for localstr, other types here?
if not isinstance(formatting, bytes):
@@ -425,46 +420,33 @@
if not isinstance(label, bytes):
raise ValueError('must use bytes for labels')
- # Formatting string must be UTF-8.
- formatting = formatting.decode(r'utf-8', r'replace').encode(r'utf-8')
+ # Formatting string must be ASCII.
+ formatting = formatting.decode(r'ascii', r'replace').encode(r'ascii')
# Arguments must be UTF-8.
args = [a.decode(r'utf-8', r'replace').encode(r'utf-8') for a in args]
# Labels must be ASCII.
labels = [l.decode(r'ascii', r'strict').encode(r'ascii')
for l in labels]
- if len(formatting) > 65535:
- raise ValueError('formatting string cannot be longer than 64k')
-
- if any(len(a) > 65535 for a in args):
- raise ValueError('argument string cannot be longer than 64k')
-
- if any(len(l) > 255 for l in labels):
- raise ValueError('label string cannot be longer than 255 bytes')
+ atom = {b'msg': formatting}
+ if args:
+ atom[b'args'] = args
+ if labels:
+ atom[b'labels'] = labels
- chunks = [
- struct.pack(r'<H', len(formatting)),
- struct.pack(r'<BB', len(labels), len(args)),
- struct.pack(r'<' + r'B' * len(labels), *map(len, labels)),
- struct.pack(r'<' + r'H' * len(args), *map(len, args)),
- ]
- chunks.append(formatting)
- chunks.extend(labels)
- chunks.extend(args)
+ atomdicts.append(atom)
- atom = b''.join(chunks)
- atomchunks.append(atom)
- bytesleft -= len(atom)
+ payload = cbor.dumps(atomdicts, canonical=True)
- if bytesleft < 0:
+ if len(payload) > maxframesize:
raise ValueError('cannot encode data in a single frame')
yield stream.makeframe(requestid=requestid,
typeid=FRAME_TYPE_TEXT_OUTPUT,
flags=0,
- payload=b''.join(atomchunks))
+ payload=payload)
class stream(object):
"""Represents a logical unidirectional series of frames."""
diff --git a/mercurial/help/internals/wireprotocol.txt b/mercurial/help/internals/wireprotocol.txt
--- a/mercurial/help/internals/wireprotocol.txt
+++ b/mercurial/help/internals/wireprotocol.txt
@@ -699,26 +699,16 @@
formatting strings with additional formatters, hence why ``%%`` is
required to represent the literal ``%``.
-The raw frame consists of a series of data structures representing
-textual atoms to print. Each atom begins with a struct defining the
-size of the data that follows:
+The frame payload consists of a CBOR array of CBOR maps. Each map
+defines an *atom* of text data to print. Each *atom* has the following
+bytestring keys:
-* A 16-bit little endian unsigned integer denoting the length of the
- formatting string.
-* An 8-bit unsigned integer denoting the number of label strings
- that follow.
-* An 8-bit unsigned integer denoting the number of formatting string
- arguments strings that follow.
-* An array of 8-bit unsigned integers denoting the lengths of
- *labels* data.
-* An array of 16-bit unsigned integers denoting the lengths of
- formatting strings.
-* The formatting string, encoded as UTF-8.
-* 0 or more ASCII strings defining labels to apply to this atom.
-* 0 or more UTF-8 strings that will be used as arguments to the
- formatting string.
-
-TODO use ASCII for formatting string.
+msg
+ (bytestring) The formatting string. Content MUST be ASCII.
+args (optional)
+ Array of bytestrings defining arguments to the formatting string.
+labels (optional)
+ Array of bytestrings defining labels to apply to this atom.
All data to be printed MUST be encoded into a single frame: this frame
does not support spanning data across multiple frames.
To: indygreg, #hg-reviewers
Cc: mercurial-devel
More information about the Mercurial-devel
mailing list