This is an archive of the discontinued Mercurial Phabricator instance.

Differential D2721

util: observable proxy objects for sockets
ClosedPublic

Authored by indygreg on Mar 8 2018, 12:31 AM.

Details

Reviewers
durin42
Group Reviewers
hg-reviewers
Commits
rHG8453699a1f21: util: observable proxy objects for sockets
Summary

We previously introduced proxy objects and observers for file objects
to help implement low-level tests for the SSH wire protocol.

In this commit, we do the same for sockets in order to help test the
HTTP server.

We only proxy/observe some socket methods. I didn't feel like
implementing all the methods because there are so many of them and
implementing them will provide no short term value. We can always
implement them later.

  1. no-check-commit because we implement foo_bar methods on stdlib types

Diff Detail

Repository
rHG Mercurial
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

indygreg created this revision.Mar 8 2018, 12:31 AM
indygreg updated this revision to Diff 6972.Mar 12 2018, 8:07 PM
indygreg planned changes to this revision.Mar 13 2018, 1:38 PM

I have significant changes to this series in flight. It's worth holding off on review.

indygreg edited parent revisions, added: D2840: hgweb: allow defining Server response header for HTTP server; removed: D2832: hgweb: remove wsgirequest (API).Mar 13 2018, 3:19 PM
indygreg requested review of this revision.

This series should now be ready to review.

mharbison72 added a subscriber: mharbison72.Mar 13 2018, 5:11 PM

fileobjectproxy needed __nonzero__()/__bool__(). Does socketproxy need it for consistency?

indygreg updated this revision to Diff 7007.Mar 13 2018, 9:15 PM
durin42 accepted this revision.Mar 19 2018, 9:36 PM
This revision is now accepted and ready to land.Mar 19 2018, 9:36 PM
Closed by commit rHG8453699a1f21: util: observable proxy objects for sockets (authored by indygreg). · Explain WhyMar 19 2018, 9:40 PM
This revision was automatically updated to reflect the committed changes.
pulkit added a subscriber: pulkit.May 20 2018, 5:08 AM
pulkit added inline comments.
mercurial/util.py
1019

If I am understanding this correctly, this can lead to TypeError: not enough arguments for format string

pulkit added inline comments.May 20 2018, 5:27 AM
mercurial/util.py
1014

While debugging the test failure on Python 3.6, the caller at line 751 can pass 5 positional arguments.

Here is the traceback after running test-wireproto-command-branchmap.t:

+  ** Unknown exception encountered with possibly-broken third-party extension drawdag
+  ** which supports versions unknown of Mercurial.
+  ** Please disable drawdag and try your action again.
+  ** If that fixes the bug please report it to the extension author.
+  ** Python 3.6.5 (default, Mar 29 2018, 03:28:50) [GCC 5.4.0 20160609]
+  ** Mercurial Distributed SCM (version 4.6+251-5a87bf0bd343)
+  ** Extensions loaded: drawdag
+  Traceback (most recent call last):
+    File "/tmp/hgtests.esettwc6/install/bin/hg", line 41, in <module>
+      dispatch.run()
+    File "/tmp/hgtests.esettwc6/install/lib/python/mercurial/dispatch.py", line 90, in run
+      status = dispatch(req)
+    File "/tmp/hgtests.esettwc6/install/lib/python/mercurial/dispatch.py", line 213, in dispatch
+      ret = _runcatch(req) or 0
+    File "/tmp/hgtests.esettwc6/install/lib/python/mercurial/dispatch.py", line 354, in _runcatch
+      return _callcatch(ui, _runcatchfunc)
+    File "/tmp/hgtests.esettwc6/install/lib/python/mercurial/dispatch.py", line 362, in _callcatch
+      return scmutil.callcatch(ui, func)
+    File "/tmp/hgtests.esettwc6/install/lib/python/mercurial/scmutil.py", line 161, in callcatch
+      return func()
+    File "/tmp/hgtests.esettwc6/install/lib/python/mercurial/dispatch.py", line 344, in _runcatchfunc
+      return _dispatch(req)
+    File "/tmp/hgtests.esettwc6/install/lib/python/mercurial/dispatch.py", line 974, in _dispatch
+      cmdpats, cmdoptions)
+    File "/tmp/hgtests.esettwc6/install/lib/python/mercurial/dispatch.py", line 730, in runcommand
+      ret = _runcommand(ui, options, cmd, d)
+    File "/tmp/hgtests.esettwc6/install/lib/python/mercurial/dispatch.py", line 982, in _runcommand
+      return cmdfunc()
+    File "/tmp/hgtests.esettwc6/install/lib/python/mercurial/dispatch.py", line 971, in <lambda>
+      d = lambda: util.checksignature(func)(ui, *args, **strcmdopt)
+    File "/tmp/hgtests.esettwc6/install/lib/python/mercurial/util.py", line 1550, in check
+      return func(*args, **kwargs)
+    File "/tmp/hgtests.esettwc6/install/lib/python/mercurial/debugcommands.py", line 2932, in debugwireproto
+      peer = httppeer.makepeer(ui, path, opener=opener)
+    File "/tmp/hgtests.esettwc6/install/lib/python/mercurial/httppeer.py", line 951, in makepeer
+      respurl, info = performhandshake(ui, url, opener, requestbuilder)
+    File "/tmp/hgtests.esettwc6/install/lib/python/mercurial/httppeer.py", line 871, in performhandshake
+      resp = sendrequest(ui, opener, req)
+    File "/tmp/hgtests.esettwc6/install/lib/python/mercurial/httppeer.py", line 311, in sendrequest
+      res = opener.open(req)
+    File "/usr/lib/python3.6/urllib/request.py", line 526, in open
+      response = self._open(req, data)
+    File "/usr/lib/python3.6/urllib/request.py", line 544, in _open
+      '_open', req)
+    File "/usr/lib/python3.6/urllib/request.py", line 504, in _call_chain
+      result = func(*args)
+    File "/tmp/hgtests.esettwc6/install/lib/python/mercurial/url.py", line 331, in http_open
+      return self.do_open(self._makeconnection, req)
+    File "/tmp/hgtests.esettwc6/install/lib/python/mercurial/keepalive.py", line 240, in do_open
+      self._start_transaction(h, req)
+    File "/tmp/hgtests.esettwc6/install/lib/python/mercurial/url.py", line 301, in _start_transaction
+      return keepalive.HTTPHandler._start_transaction(self, h, req)
+    File "/tmp/hgtests.esettwc6/install/lib/python/mercurial/keepalive.py", line 343, in _start_transaction
+      h.endheaders()
+    File "/usr/lib/python3.6/http/client.py", line 1234, in endheaders
+      self._send_output(message_body, encode_chunked=encode_chunked)
+    File "/usr/lib/python3.6/http/client.py", line 1026, in _send_output
+      self.send(msg)
+    File "/tmp/hgtests.esettwc6/install/lib/python/mercurial/keepalive.py", line 563, in safesend
+      self.connect()
+    File "/usr/lib/python3.6/http/client.py", line 937, in connect
+      self.sock.setsockopt(socket.IPPROTO_TCP, socket.TCP_NODELAY, 1)
+    File "/tmp/hgtests.esettwc6/install/lib/python/mercurial/util.py", line 738, in setsockopt
+      r'setsockopt', *args, **kwargs)
+    File "/tmp/hgtests.esettwc6/install/lib/python/mercurial/util.py", line 679, in _observedcall
+      fn(res, *args, **kwargs)
+  TypeError: setsockopt() takes 4 positional arguments but 5 were given
+  [1]

I am not sure whether this exact code path is tested on Python 2 or not. Can you have a look?

Revision Contents
Changeset List

PathPackages
Mmercurial/util.py (251 lines)

Diff 7156

mercurial/util.py

res = super(observedbufferedinputpipe, self).readline(*args, **kwargs) res = super(observedbufferedinputpipe, self).readline(*args, **kwargs)
fn = getattr(self._input._observer, r'bufferedreadline', None) fn = getattr(self._input._observer, r'bufferedreadline', None)
if fn: if fn:
fn(res) fn(res)
return res return res
PROXIED_SOCKET_METHODS = {
r'makefile',
r'recv',
r'recvfrom',
r'recvfrom_into',
r'recv_into',
r'send',
r'sendall',
r'sendto',
r'setblocking',
r'settimeout',
r'gettimeout',
r'setsockopt',
}
class socketproxy(object):
"""A proxy around a socket that tells a watcher when events occur.
This is like ``fileobjectproxy`` except for sockets.
This type is intended to only be used for testing purposes. Think hard
before using it in important code.
"""
__slots__ = (
r'_orig',
r'_observer',
)
def __init__(self, sock, observer):
object.__setattr__(self, r'_orig', sock)
object.__setattr__(self, r'_observer', observer)
def __getattribute__(self, name):
if name in PROXIED_SOCKET_METHODS:
return object.__getattribute__(self, name)
return getattr(object.__getattribute__(self, r'_orig'), name)
def __delattr__(self, name):
return delattr(object.__getattribute__(self, r'_orig'), name)
def __setattr__(self, name, value):
return setattr(object.__getattribute__(self, r'_orig'), name, value)
def __nonzero__(self):
return bool(object.__getattribute__(self, r'_orig'))
__bool__ = __nonzero__
def _observedcall(self, name, *args, **kwargs):
# Call the original object.
orig = object.__getattribute__(self, r'_orig')
res = getattr(orig, name)(*args, **kwargs)
# Call a method on the observer of the same name with arguments
# so it can react, log, etc.
observer = object.__getattribute__(self, r'_observer')
fn = getattr(observer, name, None)
if fn:
fn(res, *args, **kwargs)
return res
def makefile(self, *args, **kwargs):
res = object.__getattribute__(self, r'_observedcall')(
r'makefile', *args, **kwargs)
# The file object may be used for I/O. So we turn it into a
# proxy using our observer.
observer = object.__getattribute__(self, r'_observer')
return makeloggingfileobject(observer.fh, res, observer.name,
reads=observer.reads,
writes=observer.writes,
logdata=observer.logdata)
def recv(self, *args, **kwargs):
return object.__getattribute__(self, r'_observedcall')(
r'recv', *args, **kwargs)
def recvfrom(self, *args, **kwargs):
return object.__getattribute__(self, r'_observedcall')(
r'recvfrom', *args, **kwargs)
def recvfrom_into(self, *args, **kwargs):
return object.__getattribute__(self, r'_observedcall')(
r'recvfrom_into', *args, **kwargs)
def recv_into(self, *args, **kwargs):
return object.__getattribute__(self, r'_observedcall')(
r'recv_info', *args, **kwargs)
def send(self, *args, **kwargs):
return object.__getattribute__(self, r'_observedcall')(
r'send', *args, **kwargs)
def sendall(self, *args, **kwargs):
return object.__getattribute__(self, r'_observedcall')(
r'sendall', *args, **kwargs)
def sendto(self, *args, **kwargs):
return object.__getattribute__(self, r'_observedcall')(
r'sendto', *args, **kwargs)
def setblocking(self, *args, **kwargs):
return object.__getattribute__(self, r'_observedcall')(
r'setblocking', *args, **kwargs)
def settimeout(self, *args, **kwargs):
return object.__getattribute__(self, r'_observedcall')(
r'settimeout', *args, **kwargs)
def gettimeout(self, *args, **kwargs):
return object.__getattribute__(self, r'_observedcall')(
r'gettimeout', *args, **kwargs)
def setsockopt(self, *args, **kwargs):
return object.__getattribute__(self, r'_observedcall')(
r'setsockopt', *args, **kwargs)
DATA_ESCAPE_MAP = {pycompat.bytechr(i): br'\x%02x' % i for i in range(256)} DATA_ESCAPE_MAP = {pycompat.bytechr(i): br'\x%02x' % i for i in range(256)}
DATA_ESCAPE_MAP.update({ DATA_ESCAPE_MAP.update({
b'\\': b'\\\\', b'\\': b'\\\\',
b'\r': br'\r', b'\r': br'\r',
b'\n': br'\n', b'\n': br'\n',
}) })
DATA_ESCAPE_RE = remod.compile(br'[\x00-\x08\x0a-\x1f\\\x7f-\xff]') DATA_ESCAPE_RE = remod.compile(br'[\x00-\x08\x0a-\x1f\\\x7f-\xff]')
def escapedata(s): def escapedata(s):
if isinstance(s, bytearray): if isinstance(s, bytearray):
s = bytes(s) s = bytes(s)
return DATA_ESCAPE_RE.sub(lambda m: DATA_ESCAPE_MAP[m.group(0)], s) return DATA_ESCAPE_RE.sub(lambda m: DATA_ESCAPE_MAP[m.group(0)], s)
class fileobjectobserver(object): class baseproxyobserver(object):
"""Logs file object activity."""
def __init__(self, fh, name, reads=True, writes=True, logdata=False):
self.fh = fh
self.name = name
self.logdata = logdata
self.reads = reads
self.writes = writes
def _writedata(self, data): def _writedata(self, data):
if not self.logdata: if not self.logdata:
self.fh.write('\n') self.fh.write('\n')
self.fh.flush() self.fh.flush()
return return
# Simple case writes all data on a single line. # Simple case writes all data on a single line.
if b'\n' not in data: if b'\n' not in data:
self.fh.write(': %s\n' % escapedata(data)) self.fh.write(': %s\n' % escapedata(data))
self.fh.flush() self.fh.flush()
return return
# Data with newlines is written to multiple lines. # Data with newlines is written to multiple lines.
self.fh.write(':\n') self.fh.write(':\n')
lines = data.splitlines(True) lines = data.splitlines(True)
for line in lines: for line in lines:
self.fh.write('%s> %s\n' % (self.name, escapedata(line))) self.fh.write('%s> %s\n' % (self.name, escapedata(line)))
self.fh.flush() self.fh.flush()
class fileobjectobserver(baseproxyobserver):
"""Logs file object activity."""
def __init__(self, fh, name, reads=True, writes=True, logdata=False):
self.fh = fh
self.name = name
self.logdata = logdata
self.reads = reads
self.writes = writes
def read(self, res, size=-1): def read(self, res, size=-1):
if not self.reads: if not self.reads:
return return
# Python 3 can return None from reads at EOF instead of empty strings. # Python 3 can return None from reads at EOF instead of empty strings.
if res is None: if res is None:
res = '' res = ''
self.fh.write('%s> read(%d) -> %d' % (self.name, size, len(res))) self.fh.write('%s> read(%d) -> %d' % (self.name, size, len(res)))
def makeloggingfileobject(logh, fh, name, reads=True, writes=True, def makeloggingfileobject(logh, fh, name, reads=True, writes=True,
logdata=False): logdata=False):
"""Turn a file object into a logging file object.""" """Turn a file object into a logging file object."""
observer = fileobjectobserver(logh, name, reads=reads, writes=writes, observer = fileobjectobserver(logh, name, reads=reads, writes=writes,
logdata=logdata) logdata=logdata)
return fileobjectproxy(fh, observer) return fileobjectproxy(fh, observer)
class socketobserver(baseproxyobserver):
"""Logs socket activity."""
def __init__(self, fh, name, reads=True, writes=True, states=True,
logdata=False):
self.fh = fh
self.name = name
self.reads = reads
self.writes = writes
self.states = states
self.logdata = logdata
def makefile(self, res, mode=None, bufsize=None):
if not self.states:
return
self.fh.write('%s> makefile(%r, %r)\n' % (
self.name, mode, bufsize))
def recv(self, res, size, flags=0):
if not self.reads:
return
self.fh.write('%s> recv(%d, %d) -> %d' % (
self.name, size, flags, len(res)))
self._writedata(res)
def recvfrom(self, res, size, flags=0):
if not self.reads:
return
self.fh.write('%s> recvfrom(%d, %d) -> %d' % (
self.name, size, flags, len(res[0])))
self._writedata(res[0])
def recvfrom_into(self, res, buf, size, flags=0):
if not self.reads:
return
self.fh.write('%s> recvfrom_into(%d, %d) -> %d' % (
self.name, size, flags, res[0]))
self._writedata(buf[0:res[0]])
def recv_into(self, res, buf, size=0, flags=0):
if not self.reads:
return
self.fh.write('%s> recv_into(%d, %d) -> %d' % (
self.name, size, flags, res))
self._writedata(buf[0:res])
def send(self, res, data, flags=0):
if not self.writes:
return
self.fh.write('%s> send(%d, %d) -> %d' % (
self.name, len(data), flags, len(res)))
self._writedata(data)
def sendall(self, res, data, flags=0):
if not self.writes:
return
# Returns None on success. So don't bother reporting return value.
self.fh.write('%s> sendall(%d, %d)' % (
self.name, len(data), flags))
self._writedata(data)
def sendto(self, res, data, flagsoraddress, address=None):
if not self.writes:
return
if address:
flags = flagsoraddress
else:
flags = 0
self.fh.write('%s> sendto(%d, %d, %r) -> %d' % (
self.name, len(data), flags, address, res))
self._writedata(data)
def setblocking(self, res, flag):
if not self.states:
return
self.fh.write('%s> setblocking(%r)\n' % (self.name, flag))
def settimeout(self, res, value):
if not self.states:
return
self.fh.write('%s> settimeout(%r)\n' % (self.name, value))
def gettimeout(self, res):
if not self.states:
return
self.fh.write('%s> gettimeout() -> %f\n' % (self.name, res))
def setsockopt(self, level, optname, value):
pulkitUnsubmitted
Not Done

While debugging the test failure on Python 3.6, the caller at line 751 can pass 5 positional arguments.

Here is the traceback after running test-wireproto-command-branchmap.t:

+  ** Unknown exception encountered with possibly-broken third-party extension drawdag
+  ** which supports versions unknown of Mercurial.
+  ** Please disable drawdag and try your action again.
+  ** If that fixes the bug please report it to the extension author.
+  ** Python 3.6.5 (default, Mar 29 2018, 03:28:50) [GCC 5.4.0 20160609]
+  ** Mercurial Distributed SCM (version 4.6+251-5a87bf0bd343)
+  ** Extensions loaded: drawdag
+  Traceback (most recent call last):
+    File "/tmp/hgtests.esettwc6/install/bin/hg", line 41, in <module>
+      dispatch.run()
+    File "/tmp/hgtests.esettwc6/install/lib/python/mercurial/dispatch.py", line 90, in run
+      status = dispatch(req)
+    File "/tmp/hgtests.esettwc6/install/lib/python/mercurial/dispatch.py", line 213, in dispatch
+      ret = _runcatch(req) or 0
+    File "/tmp/hgtests.esettwc6/install/lib/python/mercurial/dispatch.py", line 354, in _runcatch
+      return _callcatch(ui, _runcatchfunc)
+    File "/tmp/hgtests.esettwc6/install/lib/python/mercurial/dispatch.py", line 362, in _callcatch
+      return scmutil.callcatch(ui, func)
+    File "/tmp/hgtests.esettwc6/install/lib/python/mercurial/scmutil.py", line 161, in callcatch
+      return func()
+    File "/tmp/hgtests.esettwc6/install/lib/python/mercurial/dispatch.py", line 344, in _runcatchfunc
+      return _dispatch(req)
+    File "/tmp/hgtests.esettwc6/install/lib/python/mercurial/dispatch.py", line 974, in _dispatch
+      cmdpats, cmdoptions)
+    File "/tmp/hgtests.esettwc6/install/lib/python/mercurial/dispatch.py", line 730, in runcommand
+      ret = _runcommand(ui, options, cmd, d)
+    File "/tmp/hgtests.esettwc6/install/lib/python/mercurial/dispatch.py", line 982, in _runcommand
+      return cmdfunc()
+    File "/tmp/hgtests.esettwc6/install/lib/python/mercurial/dispatch.py", line 971, in <lambda>
+      d = lambda: util.checksignature(func)(ui, *args, **strcmdopt)
+    File "/tmp/hgtests.esettwc6/install/lib/python/mercurial/util.py", line 1550, in check
+      return func(*args, **kwargs)
+    File "/tmp/hgtests.esettwc6/install/lib/python/mercurial/debugcommands.py", line 2932, in debugwireproto
+      peer = httppeer.makepeer(ui, path, opener=opener)
+    File "/tmp/hgtests.esettwc6/install/lib/python/mercurial/httppeer.py", line 951, in makepeer
+      respurl, info = performhandshake(ui, url, opener, requestbuilder)
+    File "/tmp/hgtests.esettwc6/install/lib/python/mercurial/httppeer.py", line 871, in performhandshake
+      resp = sendrequest(ui, opener, req)
+    File "/tmp/hgtests.esettwc6/install/lib/python/mercurial/httppeer.py", line 311, in sendrequest
+      res = opener.open(req)
+    File "/usr/lib/python3.6/urllib/request.py", line 526, in open
+      response = self._open(req, data)
+    File "/usr/lib/python3.6/urllib/request.py", line 544, in _open
+      '_open', req)
+    File "/usr/lib/python3.6/urllib/request.py", line 504, in _call_chain
+      result = func(*args)
+    File "/tmp/hgtests.esettwc6/install/lib/python/mercurial/url.py", line 331, in http_open
+      return self.do_open(self._makeconnection, req)
+    File "/tmp/hgtests.esettwc6/install/lib/python/mercurial/keepalive.py", line 240, in do_open
+      self._start_transaction(h, req)
+    File "/tmp/hgtests.esettwc6/install/lib/python/mercurial/url.py", line 301, in _start_transaction
+      return keepalive.HTTPHandler._start_transaction(self, h, req)
+    File "/tmp/hgtests.esettwc6/install/lib/python/mercurial/keepalive.py", line 343, in _start_transaction
+      h.endheaders()
+    File "/usr/lib/python3.6/http/client.py", line 1234, in endheaders
+      self._send_output(message_body, encode_chunked=encode_chunked)
+    File "/usr/lib/python3.6/http/client.py", line 1026, in _send_output
+      self.send(msg)
+    File "/tmp/hgtests.esettwc6/install/lib/python/mercurial/keepalive.py", line 563, in safesend
+      self.connect()
+    File "/usr/lib/python3.6/http/client.py", line 937, in connect
+      self.sock.setsockopt(socket.IPPROTO_TCP, socket.TCP_NODELAY, 1)
+    File "/tmp/hgtests.esettwc6/install/lib/python/mercurial/util.py", line 738, in setsockopt
+      r'setsockopt', *args, **kwargs)
+    File "/tmp/hgtests.esettwc6/install/lib/python/mercurial/util.py", line 679, in _observedcall
+      fn(res, *args, **kwargs)
+  TypeError: setsockopt() takes 4 positional arguments but 5 were given
+  [1]

I am not sure whether this exact code path is tested on Python 2 or not. Can you have a look?

pulkit: While debugging the test failure on Python 3.6, the caller at line 751 can pass 5 positional…
if not self.states:
return
self.fh.write('%s> setsockopt(%r, %r, %r) -> %r\n' % (
self.name, level, optname, value))
pulkitUnsubmitted
Not Done

If I am understanding this correctly, this can lead to TypeError: not enough arguments for format string

pulkit: If I am understanding this correctly, this can lead to `TypeError: not enough arguments for…
def makeloggingsocket(logh, fh, name, reads=True, writes=True, states=True,
logdata=False):
"""Turn a socket into a logging socket."""
observer = socketobserver(logh, name, reads=reads, writes=writes,
states=states, logdata=logdata)
return socketproxy(fh, observer)
def version(): def version():
"""Return version information if available.""" """Return version information if available."""
try: try:
from . import __version__ from . import __version__
return __version__.version return __version__.version
except ImportError: except ImportError:
return 'unknown' return 'unknown'