From 8cad3ffeac28e6af46b67d0552b6bf943fc9a88d Mon Sep 17 00:00:00 2001 From: Christian Decker Date: Mon, 16 Dec 2019 14:54:22 +0100 Subject: [PATCH] pyln: Work around the socket path length on Linux OSs Some Linux OSs impose a length limit on the path a Unix socket may have. This is not an issue in `lightningd` since we `chdir()` into that directory before opening the socket, however in pyln this became a problem for some tests, since we use absolute paths in the testing framework. It's also a rather strange quirk to expose to users. This patch introduces a `UnixSocket` abstraction that attempts to work around these limitations by aliasing the directory containing the socket into `/proc/self/fd` and then connecting using that alias. It was inspired by Open vSwitch code here https://github.com/openvswitch/ovs/blob/master/python/ovs/socket_util.py Signed-off-by: Christian Decker <@cdecker> --- contrib/pyln-client/pyln/client/lightning.py | 75 ++++++++++++++++++-- tests/test_misc.py | 21 +++++- 2 files changed, 90 insertions(+), 6 deletions(-) diff --git a/contrib/pyln-client/pyln/client/lightning.py b/contrib/pyln-client/pyln/client/lightning.py index 7c0d49687..a943c9905 100644 --- a/contrib/pyln-client/pyln/client/lightning.py +++ b/contrib/pyln-client/pyln/client/lightning.py @@ -1,9 +1,10 @@ +from decimal import Decimal +from math import floor, log10 import json import logging +import os import socket import warnings -from decimal import Decimal -from math import floor, log10 class RpcError(ValueError): @@ -159,6 +160,73 @@ class Millisatoshi: return Millisatoshi(int(self) + int(other)) +class UnixSocket(object): + """A wrapper for socket.socket that is specialized to unix sockets. + + Some OS implementations impose restrictions on the Unix sockets. + + - On linux OSs the socket path must be shorter than the in-kernel buffer + size (somewhere around 100 bytes), thus long paths may end up failing + the `socket.connect` call. + + This is a small wrapper that tries to work around these limitations. + + """ + + def __init__(self, path): + self.path = path + self.sock = None + self.connect() + + def connect(self): + try: + self.sock = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM) + return self.sock.connect(self.path) + except OSError as e: + self.sock.close() + + if (e.args[0] == "AF_UNIX path too long" and os.uname()[0] == "Linux"): + # If this is a Linux system we may be able to work around this + # issue by opening our directory and using `/proc/self/fd/` to + # get a short alias for the socket file. + # + # This was heavily inspired by the Open vSwitch code see here: + # https://github.com/openvswitch/ovs/blob/master/python/ovs/socket_util.py + + dirname = os.path.dirname(self.path) + basename = os.path.basename(self.path) + + # Open an fd to our home directory, that we can then find + # through `/proc/self/fd` and access the contents. + dirfd = os.open(dirname, os.O_DIRECTORY | os.O_RDONLY) + short_path = "/proc/self/fd/%d/%s" % (dirfd, basename) + self.sock = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM) + return self.sock.connect(short_path) + else: + # There is no good way to recover from this. + raise + + def close(self): + if self.sock is not None: + self.sock.close() + self.sock = None + + def sendall(self, b): + if self.sock is None: + raise socket.error("not connected") + + self.sock.sendall(b) + + def recv(self, length): + if self.sock is None: + raise socket.error("not connected") + + return self.sock.recv(length) + + def __del__(self): + self.close() + + class UnixDomainSocketRpc(object): def __init__(self, socket_path, executor=None, logger=logging, encoder_cls=json.JSONEncoder, decoder=json.JSONDecoder()): self.socket_path = socket_path @@ -215,8 +283,7 @@ class UnixDomainSocketRpc(object): payload = {k: v for k, v in payload.items() if v is not None} # FIXME: we open a new socket for every readobj call... - sock = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM) - sock.connect(self.socket_path) + sock = UnixSocket(self.socket_path) self._writeobj(sock, { "method": method, "params": payload, diff --git a/tests/test_misc.py b/tests/test_misc.py index 0c4b28255..a0b4e4bde 100644 --- a/tests/test_misc.py +++ b/tests/test_misc.py @@ -1,11 +1,11 @@ from bitcoin.rpc import RawProxy from decimal import Decimal from fixtures import * # noqa: F401,F403 -from fixtures import TEST_NETWORK +from fixtures import LightningNode, TEST_NETWORK from flaky import flaky # noqa: F401 from lightning import RpcError from threading import Event -from utils import DEVELOPER, TIMEOUT, VALGRIND, sync_blockheight, only_one, wait_for, TailableProc, env +from pyln.testing.utils import DEVELOPER, TIMEOUT, VALGRIND, sync_blockheight, only_one, wait_for, TailableProc, env from ephemeral_port_reserve import reserve import json @@ -2002,3 +2002,20 @@ def test_unicode_rpc(node_factory): assert(len(invoices) == 1) assert(invoices[0]['description'] == desc) assert(invoices[0]['label'] == desc) + + +@unittest.skipIf(VALGRIND, "Testing pyln doesn't exercise anything interesting in the c code.") +def test_unix_socket_path_length(node_factory, bitcoind, directory, executor, db_provider, test_base_dir): + lightning_dir = os.path.join(directory, "anode" + "far" * 30 + "away") + os.makedirs(lightning_dir) + db = db_provider.get_db(lightning_dir, "test_unix_socket_path_length", 1) + + l1 = LightningNode(1, lightning_dir, bitcoind, executor, db=db, port=node_factory.get_next_port()) + + # `LightningNode.start()` internally calls `LightningRpc.getinfo()` which + # exercises the socket logic, and raises an issue if it fails. + l1.start() + + # Let's just call it again to make sure it really works. + l1.rpc.listconfigs() + l1.stop()