This is an archive of the discontinued Mercurial Phabricator instance.

Differential D4466

wireprotoframing: use our CBOR module
ClosedPublic

Authored by indygreg on Sep 4 2018, 2:28 PM.

Details

Reviewers
pulkit
Group Reviewers
hg-reviewers
Commits
rHG36f487a332ad: wireprotoframing: use our CBOR module
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.

Diff Detail

Repository
rHG Mercurial
Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

indygreg created this revision.Sep 4 2018, 2:28 PM
pulkit accepted this revision.Sep 5 2018, 7:58 AM
Closed by commit rHG36f487a332ad: wireprotoframing: use our CBOR module (authored by indygreg). · Explain WhySep 5 2018, 8:23 AM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

Diff 10763

mercurial/wireprotoframing.py

from __future__ import absolute_import from __future__ import absolute_import
import collections import collections
import struct import struct
from .i18n import _ from .i18n import _
from .thirdparty import ( from .thirdparty import (
attr, attr,
cbor,
) )
from . import ( from . import (
encoding, encoding,
error, error,
util, util,
) )
from .utils import ( from .utils import (
cborutil,
stringutil, stringutil,
) )
FRAME_HEADER_SIZE = 8 FRAME_HEADER_SIZE = 8
DEFAULT_MAX_FRAME_SIZE = 32768 DEFAULT_MAX_FRAME_SIZE = 32768
STREAM_FLAG_BEGIN_STREAM = 0x01 STREAM_FLAG_BEGIN_STREAM = 0x01
STREAM_FLAG_END_STREAM = 0x02 STREAM_FLAG_END_STREAM = 0x02
validflags = FRAME_TYPE_FLAGS[frametype] validflags = FRAME_TYPE_FLAGS[frametype]
for flag in frameflags.split(b'|'): for flag in frameflags.split(b'|'):
if flag in validflags: if flag in validflags:
finalflags |= validflags[flag] finalflags |= validflags[flag]
else: else:
finalflags |= int(flag) finalflags |= int(flag)
if payload.startswith(b'cbor:'): if payload.startswith(b'cbor:'):
payload = cbor.dumps(stringutil.evalpythonliteral(payload[5:]), payload = b''.join(cborutil.streamencode(
canonical=True) stringutil.evalpythonliteral(payload[5:])))
else: else:
payload = stringutil.unescapestr(payload) payload = stringutil.unescapestr(payload)
return makeframe(requestid=requestid, streamid=streamid, return makeframe(requestid=requestid, streamid=streamid,
streamflags=finalstreamflags, typeid=frametype, streamflags=finalstreamflags, typeid=frametype,
flags=finalflags, payload=payload) flags=finalflags, payload=payload)
This is a generator of bytearrays. Each item represents a frame This is a generator of bytearrays. Each item represents a frame
ready to be sent over the wire to a peer. ready to be sent over the wire to a peer.
""" """
data = {b'name': cmd} data = {b'name': cmd}
if args: if args:
data[b'args'] = args data[b'args'] = args
data = cbor.dumps(data, canonical=True) data = b''.join(cborutil.streamencode(data))
offset = 0 offset = 0
while True: while True:
flags = 0 flags = 0
# Must set new or continuation flag. # Must set new or continuation flag.
if not offset: if not offset:
def createcommandresponseframesfrombytes(stream, requestid, data, def createcommandresponseframesfrombytes(stream, requestid, data,
maxframesize=DEFAULT_MAX_FRAME_SIZE): maxframesize=DEFAULT_MAX_FRAME_SIZE):
"""Create a raw frame to send a bytes response from static bytes input. """Create a raw frame to send a bytes response from static bytes input.
Returns a generator of bytearrays. Returns a generator of bytearrays.
""" """
# Automatically send the overall CBOR response map. # 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: if len(overall) > maxframesize:
raise error.ProgrammingError('not yet implemented') raise error.ProgrammingError('not yet implemented')
# Simple case where we can fit the full response in a single frame. # Simple case where we can fit the full response in a single frame.
if len(overall) + len(data) <= maxframesize: if len(overall) + len(data) <= maxframesize:
flags = FLAG_COMMAND_RESPONSE_EOS flags = FLAG_COMMAND_RESPONSE_EOS
yield stream.makeframe(requestid=requestid, yield stream.makeframe(requestid=requestid,
typeid=FRAME_TYPE_COMMAND_RESPONSE, typeid=FRAME_TYPE_COMMAND_RESPONSE,
flags=flags, flags=flags,
payload=chunk) payload=chunk)
if done: if done:
break break
def createbytesresponseframesfromgen(stream, requestid, gen, def createbytesresponseframesfromgen(stream, requestid, gen,
maxframesize=DEFAULT_MAX_FRAME_SIZE): 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, yield stream.makeframe(requestid=requestid,
typeid=FRAME_TYPE_COMMAND_RESPONSE, typeid=FRAME_TYPE_COMMAND_RESPONSE,
flags=FLAG_COMMAND_RESPONSE_CONTINUATION, flags=FLAG_COMMAND_RESPONSE_CONTINUATION,
payload=overall) payload=overall)
cb = util.chunkbuffer(gen) cb = util.chunkbuffer(gen)
b'error': { b'error': {
b'message': message, b'message': message,
} }
} }
if args: if args:
m[b'error'][b'args'] = args m[b'error'][b'args'] = args
overall = cbor.dumps(m, canonical=True) overall = b''.join(cborutil.streamencode(m))
yield stream.makeframe(requestid=requestid, yield stream.makeframe(requestid=requestid,
typeid=FRAME_TYPE_COMMAND_RESPONSE, typeid=FRAME_TYPE_COMMAND_RESPONSE,
flags=FLAG_COMMAND_RESPONSE_EOS, flags=FLAG_COMMAND_RESPONSE_EOS,
payload=overall) payload=overall)
def createerrorframe(stream, requestid, msg, errtype): def createerrorframe(stream, requestid, msg, errtype):
# TODO properly handle frame size limits. # TODO properly handle frame size limits.
assert len(msg) <= DEFAULT_MAX_FRAME_SIZE assert len(msg) <= DEFAULT_MAX_FRAME_SIZE
payload = cbor.dumps({ payload = b''.join(cborutil.streamencode({
b'type': errtype, b'type': errtype,
b'message': [{b'msg': msg}], b'message': [{b'msg': msg}],
}, canonical=True) }))
yield stream.makeframe(requestid=requestid, yield stream.makeframe(requestid=requestid,
typeid=FRAME_TYPE_ERROR_RESPONSE, typeid=FRAME_TYPE_ERROR_RESPONSE,
flags=0, flags=0,
payload=payload) payload=payload)
def createtextoutputframe(stream, requestid, atoms, def createtextoutputframe(stream, requestid, atoms,
maxframesize=DEFAULT_MAX_FRAME_SIZE): maxframesize=DEFAULT_MAX_FRAME_SIZE):
atom = {b'msg': formatting} atom = {b'msg': formatting}
if args: if args:
atom[b'args'] = args atom[b'args'] = args
if labels: if labels:
atom[b'labels'] = labels atom[b'labels'] = labels
atomdicts.append(atom) atomdicts.append(atom)
payload = cbor.dumps(atomdicts, canonical=True) payload = b''.join(cborutil.streamencode(atomdicts))
if len(payload) > maxframesize: if len(payload) > maxframesize:
raise ValueError('cannot encode data in a single frame') raise ValueError('cannot encode data in a single frame')
yield stream.makeframe(requestid=requestid, yield stream.makeframe(requestid=requestid,
typeid=FRAME_TYPE_TEXT_OUTPUT, typeid=FRAME_TYPE_TEXT_OUTPUT,
flags=0, flags=0,
payload=payload) payload=payload)
if self._receivingcommands: if self._receivingcommands:
self._state = 'command-receiving' self._state = 'command-receiving'
else: else:
self._state = 'idle' self._state = 'idle'
# Decode the payloads as CBOR. # Decode the payloads as CBOR.
entry['payload'].seek(0) entry['payload'].seek(0)
request = cbor.load(entry['payload']) request = cborutil.decodeall(entry['payload'].getvalue())[0]
if b'name' not in request: if b'name' not in request:
self._state = 'errored' self._state = 'errored'
return self._makeerrorresult( return self._makeerrorresult(
_('command request missing "name" field')) _('command request missing "name" field'))
if b'args' not in request: if b'args' not in request:
request[b'args'] = {} request[b'args'] = {}
'data': frame.payload, 'data': frame.payload,
} }
def _onerrorresponseframe(self, request, frame): def _onerrorresponseframe(self, request, frame):
request.state = 'errored' request.state = 'errored'
del self._activerequests[request.requestid] del self._activerequests[request.requestid]
# The payload should be a CBOR map. # The payload should be a CBOR map.
m = cbor.loads(frame.payload) m = cborutil.decodeall(frame.payload)[0]
return 'error', { return 'error', {
'request': request, 'request': request,
'type': m['type'], 'type': m['type'],
'message': m['message'], 'message': m['message'],
} }

tests/test-http-api-httpv2.t

s> POST /api/exp-http-v2-0001/ro/debugreflect HTTP/1.1\r\n s> POST /api/exp-http-v2-0001/ro/debugreflect HTTP/1.1\r\n
s> Accept-Encoding: identity\r\n s> Accept-Encoding: identity\r\n
s> accept: application/mercurial-exp-framing-0005\r\n s> accept: application/mercurial-exp-framing-0005\r\n
s> content-type: application/mercurial-exp-framing-0005\r\n s> content-type: application/mercurial-exp-framing-0005\r\n
s> user-agent: test\r\n s> user-agent: test\r\n
s> content-length: 47\r\n s> content-length: 47\r\n
s> host: $LOCALIP:$HGPORT\r\n (glob) s> host: $LOCALIP:$HGPORT\r\n (glob)
s> \r\n 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> makefile('rb', None)
s> HTTP/1.1 200 OK\r\n s> HTTP/1.1 200 OK\r\n
s> Server: testing stub value\r\n s> Server: testing stub value\r\n
s> Date: $HTTP_DATE$\r\n s> Date: $HTTP_DATE$\r\n
s> Content-Type: text/plain\r\n s> Content-Type: text/plain\r\n
s> Content-Length: 205\r\n s> Content-Length: 205\r\n
s> \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> ["runcommand", {"args": {"bar1": "val", "foo": "val1"}, "command": "command1", "data": null, "requestid": 1}]\n
s> received: <no frame>\n s> received: <no frame>\n
s> {"action": "noop"} s> {"action": "noop"}
Multiple requests to regular command URL are not allowed Multiple requests to regular command URL are not allowed
$ sendhttpraw << EOF $ sendhttpraw << EOF
> httprequest POST api/$HTTPV2/ro/customreadonly > httprequest POST api/$HTTPV2/ro/customreadonly

tests/test-wireproto-command-pushkey.t

s> *\r\n (glob) s> *\r\n (glob)
s> Accept-Encoding: identity\r\n s> Accept-Encoding: identity\r\n
s> accept: application/mercurial-exp-framing-0005\r\n s> accept: application/mercurial-exp-framing-0005\r\n
s> content-type: application/mercurial-exp-framing-0005\r\n s> content-type: application/mercurial-exp-framing-0005\r\n
s> content-length: 105\r\n s> content-length: 105\r\n
s> host: $LOCALIP:$HGPORT\r\n (glob) s> host: $LOCALIP:$HGPORT\r\n (glob)
s> user-agent: Mercurial debugwireproto\r\n s> user-agent: Mercurial debugwireproto\r\n
s> \r\n s> \r\n
s> a\x00\x00\x01\x00\x01\x01\x11\xa2Dargs\xa4CkeyA@CnewX(426bada5c67598ca65036d57d9e4b64b0c1ce7a0Cold@InamespaceIbookmarksDnameGpushkey s> a\x00\x00\x01\x00\x01\x01\x11\xa2Dargs\xa4CkeyA@InamespaceIbookmarksCnewX(426bada5c67598ca65036d57d9e4b64b0c1ce7a0Cold@DnameGpushkey
s> makefile('rb', None) s> makefile('rb', None)
s> HTTP/1.1 200 OK\r\n s> HTTP/1.1 200 OK\r\n
s> Server: testing stub value\r\n s> Server: testing stub value\r\n
s> Date: $HTTP_DATE$\r\n s> Date: $HTTP_DATE$\r\n
s> Content-Type: application/mercurial-exp-framing-0005\r\n s> Content-Type: application/mercurial-exp-framing-0005\r\n
s> Transfer-Encoding: chunked\r\n s> Transfer-Encoding: chunked\r\n
s> \r\n s> \r\n
s> 14\r\n s> 14\r\n