From 0d5a080425bd85d60f1b2f26886d15ba214acfc2 Mon Sep 17 00:00:00 2001 From: Timothy Allen Date: Sat, 10 Oct 2026 10:59:34 +0200 Subject: [PATCH] Read each request in full read_request called recv once for the length and once for the body. A stream socket can deliver a request in several pieces, and the server then rejected that request. The server now uses the read loop of the client, which moves to utilities as recv_exact. The server refuses a request above 64 KiB. --- bmspy/client.py | 17 +++-------------- bmspy/server.py | 13 ++++++++++--- bmspy/utilities.py | 14 ++++++++++++++ tests/test_server.py | 40 ++++++++++++++++++++++++++++++++++++++++ 4 files changed, 67 insertions(+), 17 deletions(-) diff --git a/bmspy/client.py b/bmspy/client.py index bffa3de..ff6679c 100644 --- a/bmspy/client.py +++ b/bmspy/client.py @@ -7,7 +7,7 @@ import json import socket from typing import Any -from bmspy.utilities import debugger +from bmspy.utilities import debugger, recv_exact from bmspy.classes import UPS @@ -45,17 +45,6 @@ def handle_registration(socket_path: str, client_name: str, debug: int = 0) -> d return data -def _recv_exact(sock: socket.socket, length: int) -> bytes: - """Read a complete frame, even when the stream splits it across receives.""" - data = bytearray() - while len(data) < length: - chunk = sock.recv(length - len(data)) - if not chunk: - raise OSError("Daemon closed the socket before the response was complete") - data.extend(chunk) - return bytes(data) - - def socket_comms(socket_path: str, request_data: dict[str, Any], debug: int = 0) -> dict[str, Any]: """Send a JSON request to the daemon socket and return the decoded JSON response.""" response_data: dict[str, Any] = dict() @@ -101,7 +90,7 @@ def socket_comms(socket_path: str, request_data: dict[str, Any], debug: int = 0) # get length of expected json string try: sock.sendall(request) - response = _recv_exact(sock, struct.calcsize("!I")) + response = recv_exact(sock, struct.calcsize("!I")) length = struct.unpack("!I", response)[0] if debug > 4: debugger( @@ -110,7 +99,7 @@ def socket_comms(socket_path: str, request_data: dict[str, Any], debug: int = 0) ) ) # read length bytes - response = _recv_exact(sock, length) + response = recv_exact(sock, length) if debug > 3: debugger("socket client: incoming response: {}".format(response)) response_data = json.loads(response) diff --git a/bmspy/server.py b/bmspy/server.py index b4c375a..5f3a4ba 100755 --- a/bmspy/server.py +++ b/bmspy/server.py @@ -19,7 +19,7 @@ from dataclasses import asdict as dataclass_asdict, dataclass, field from types import FrameType from typing import Any, NoReturn -from bmspy.utilities import debugger +from bmspy.utilities import debugger, recv_exact from bmspy.classes import UPS from bmspy.jbd_bms import collect_data, initialise_serial from bmspy.ecoflow_bt import ( @@ -46,6 +46,8 @@ BLUETOOTH_POLL_SECONDS = 60 FIRST_POLL_TIMEOUT = 120 # Time limit for each read from a client and each write to it. CLIENT_TIMEOUT_SECONDS = 5 +# A request is a short JSON object. A larger length prefix is an error. +MAX_REQUEST_BYTES = 64 * 1024 # One adapter: collect from one Bluetooth device at a time. bluetooth_lock = threading.Lock() @@ -82,7 +84,7 @@ def read_request(connection: socket.socket, debug: int = 0) -> dict[str, Any]: # get length of expected json string request = bytes() try: - request = connection.recv(struct.calcsize("!I")) + request = recv_exact(connection, struct.calcsize("!I")) except Exception as e: raise OSError("unable to read request length from socket: {}".format(e)) try: @@ -92,9 +94,14 @@ def read_request(connection: socket.socket, debug: int = 0) -> dict[str, Any]: if debug > 4: debugger("socket: incoming length: {}, encoded as {}".format(length, request)) + if length > MAX_REQUEST_BYTES: + raise OSError( + "request of {} bytes exceeds the limit of {}".format(length, MAX_REQUEST_BYTES) + ) + # read length bytes try: - request = connection.recv(length) + request = recv_exact(connection, length) except Exception as e: raise OSError("unable to read socket: {}".format(e)) if debug > 3: diff --git a/bmspy/utilities.py b/bmspy/utilities.py index 1640943..4525704 100755 --- a/bmspy/utilities.py +++ b/bmspy/utilities.py @@ -4,6 +4,7 @@ # import datetime import pprint +import socket from typing import Any @@ -19,3 +20,16 @@ def debugger(data: Any, pretty: bool = False) -> None: ) msg = f"{str(datetime.datetime.now())} {data}" print(msg) + + +def recv_exact(sock: socket.socket, length: int) -> bytes: + """Read a complete frame, even when the stream splits it across receives.""" + data = bytearray() + while len(data) < length: + chunk = sock.recv(length - len(data)) + if not chunk: + raise OSError( + "socket closed after {} of {} bytes".format(len(data), length) + ) + data.extend(chunk) + return bytes(data) diff --git a/tests/test_server.py b/tests/test_server.py index 1cf903c..f732ac5 100644 --- a/tests/test_server.py +++ b/tests/test_server.py @@ -253,6 +253,46 @@ class TestReadRequestErrors: srv.close() cli.close() + @pytest.mark.parametrize("split", [2, 10], ids=["inside-the-length", "inside-the-body"]) + def test_request_in_two_pieces_is_read_whole(self, split): + """A stream socket can deliver one request in several pieces.""" + srv, cli = socket_module.socketpair(socket_module.AF_UNIX, socket_module.SOCK_STREAM) + payload = json.dumps({"command": "GET", "client": "split"}).encode() + frame = struct.pack("!I", len(payload)) + payload + late = threading.Timer(0.05, cli.sendall, args=(frame[split:],)) + try: + srv.settimeout(5) + cli.sendall(frame[:split]) + late.start() + assert read_request(srv) == {"command": "GET", "client": "split"} + finally: + late.join() + srv.close() + cli.close() + + def test_client_that_closes_after_half_a_request_is_an_error(self): + srv, cli = socket_module.socketpair(socket_module.AF_UNIX, socket_module.SOCK_STREAM) + payload = json.dumps({"command": "GET", "client": "half"}).encode() + try: + cli.sendall(struct.pack("!I", len(payload)) + payload[:10]) + cli.close() + with pytest.raises(OSError, match="unable to read socket: socket closed after 10 of"): + read_request(srv) + finally: + srv.close() + + def test_request_above_the_size_limit_is_refused(self): + """A wrong length prefix must not make the server wait for gigabytes.""" + from bmspy.server import MAX_REQUEST_BYTES + srv, cli = socket_module.socketpair(socket_module.AF_UNIX, socket_module.SOCK_STREAM) + try: + cli.sendall(struct.pack("!I", MAX_REQUEST_BYTES + 1) + b"{}") + with pytest.raises(OSError, match="exceeds the limit"): + read_request(srv) + finally: + srv.close() + cli.close() + def test_truncated_length_bytes_raises(self): srv, cli = socket_module.socketpair() try: