vici: Generalize timeout support in Python bindings

Since 3dd5dc50119d ("Merge branch 'vici-python-timeout'"), any timeout
set directly on the socket is reset by `vici.Transport.receive()` unless
called by `vici.Session.listen()`. This prevents configuring a default
timeout directly on the socket.

However, setting a timeout directly on the socket also had drawbacks
since it can cause `vici.Transport.receive()` to raise a timeout error
while a subset of the data have been received, with no way to recover.

This commit merges both approaches by considering the timeout configured
on the socket by default (when no timeout is explicitly set) and
keeping the switch to blocking receive once the first byte has been
received. When the full expected data have been received, the timeout
configured on the socket is restored.

Co-authored-by: Tobias Brunner <tobias@strongswan.org>
Signed-off-by: Jean-Tiare Le Bigot <jt@yadutaf.fr>
Closes strongswan/strongswan#1562

diff --git a/src/libcharon/plugins/vici/python/vici/protocol.py b/src/libcharon/plugins/vici/python/vici/protocol.py
index 8793651882d6a4318c40aa6b34f2de691b20dbdf..45aec5312a8d434b6e37f56d25ca7e1c2bfb2d8f 100644 (file)
--- a/src/libcharon/plugins/vici/python/vici/protocol.py
+++ b/src/libcharon/plugins/vici/python/vici/protocol.py
@@ -8,6 +8,9 @@ from collections import OrderedDict
 from .exception import DeserializationException
 
 
+RECV_TIMEOUT_DEFAULT = object()
+
+
 class Transport(object):
     HEADER_LENGTH = 4
     MAX_SEGMENT = 512 * 1024
@@ -18,7 +21,7 @@ class Transport(object):
     def send(self, packet):
         self.socket.sendall(struct.pack("!I", len(packet)) + packet)
 
-    def receive(self, timeout=None):
+    def receive(self, timeout=RECV_TIMEOUT_DEFAULT):
         raw_length = self._recvall(self.HEADER_LENGTH, timeout)
         length, = struct.unpack("!I", raw_length)
         payload = self._recvall(length)
@@ -28,17 +31,21 @@ class Transport(object):
         self.socket.shutdown(socket.SHUT_RDWR)
         self.socket.close()
 
-    def _recvall(self, count, timeout=None):
+    def _recvall(self, count, timeout=RECV_TIMEOUT_DEFAULT):
         """Ensure to read count bytes from the socket"""
         data = b""
-        if count > 0:
+        old_timeout = self.socket.gettimeout()
+        if timeout is not RECV_TIMEOUT_DEFAULT:
             self.socket.settimeout(timeout)
-        while len(data) < count:
-            buf = self.socket.recv(count - len(data))
-            self.socket.settimeout(None)
-            if not buf:
-                raise socket.error('Connection closed')
-            data += buf
+        try:
+            while len(data) < count:
+                buf = self.socket.recv(count - len(data))
+                self.socket.settimeout(None)
+                if not buf:
+                    raise socket.error('Connection closed')
+                data += buf
+        finally:
+            self.socket.settimeout(old_timeout)
         return data
 
 

diff --git a/src/libcharon/plugins/vici/python/vici/session.py b/src/libcharon/plugins/vici/python/vici/session.py
index 79f2bda8150fc7e6113bcb2a72f0f8995fd0c17b..f45ff952e5ad4f5960adc198327af8fa4b75c46b 100644 (file)
--- a/src/libcharon/plugins/vici/python/vici/session.py
+++ b/src/libcharon/plugins/vici/python/vici/session.py
@@ -1,12 +1,30 @@
 import socket
 
 from .exception import SessionException, CommandException, EventUnknownException
-from .protocol import Transport, Packet, Message
+from .protocol import Transport, Packet, Message, RECV_TIMEOUT_DEFAULT
 from .command_wrappers import CommandWrappers
 
 
 class Session(CommandWrappers, object):
     def __init__(self, sock=None):
+        """Establish a session with an IKE daemon.
+
+        By default, the session will connect to the `/var/run/charon.vici` Unix
+        domain socket.
+
+        If there is a need to connect a socket in another location or set
+        specific settings on the socket (like a timeout), create and connect
+        a socket and pass it to the `sock` parameter.
+
+        .. note::
+
+            In case a timeout is set on the socket, the internal read code
+            will temporarily disable it after receiving the first byte to avoid
+            partial read corruptions.
+
+        :param sock: socket connected to the IKE daemon (optional)
+        :type sock: socket.socket
+        """
         if sock is None:
             sock = socket.socket(socket.AF_UNIX)
             sock.connect("/var/run/charon.vici")
@@ -141,7 +159,7 @@ class Session(CommandWrappers, object):
                     )
                 )
 
-    def listen(self, event_types, timeout=None):
+    def listen(self, event_types, timeout=RECV_TIMEOUT_DEFAULT):
         """Register and listen for the given events.
 
         If a timeout is given, the generator produces a (None, None) tuple