fix: --stall-after Budget der Rettungsstation client-kumulativ zählen

Der Testschalter zählte bisher pro Anfrage, sodass eine Lampe nach ihrem
Empfangs-Timeout einfach per Range neu verband und ein frisches Budget bekam
– am EVK reichten fünf Reconnects, um trotz --stall-after das komplette
Image zu übertragen. Station verfolgt die ausgelieferten Bytes jetzt pro
Client-IP über alle Anfragen hinweg (Station.reserve_stall_bytes, unter dem
bestehenden Lock), sodass kein erneutes Verbinden das Budget auffüllt.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

tools/station/server.py

@@ -61,16 +61,35 @@ class Station:
         self.since = now_iso()
         self.served = 0
         self.recent: list[dict] = []
+        # Cumulative bytes already delivered to each client while --stall-after is active,
+        # keyed by client IP. A lamp whose download stalls re-opens a fresh connection and
+        # resumes with Range right where it left off, so a per-request counter would hand
+        # every reconnect a brand-new --stall-after allowance and the stall would never
+        # actually stick (see reserve_stall_bytes below).
+        self.stall_sent: dict[str, int] = {}
         # ThreadingHTTPServer runs one handler thread per connection, and several lamps
-        # can be downloading at once. Every mutation of served/recent and the snapshot
+        # can be downloading at once. Every mutation of served/recent/stall_sent and the
-        # that gets serialized to disk must happen under this same lock, or concurrent
+        # snapshot that gets serialized to disk must happen under this same lock, or
-        # requests lose increments and stomp on each other's status.json writes.
+        # concurrent requests lose increments and stomp on each other's status.json writes.
         self.lock = threading.RLock()
         self.write_state()
 
     def image_path(self, target: str) -> Path:
         return self.image_dir / f"ninlight_{target}_ota.img"
 
+    def reserve_stall_bytes(self, ip: str, n: int) -> int:
+        """How many of the next `n` bytes this client may still receive under --stall-after.
+
+        Tracks bytes cumulatively per client IP, across requests, so a stalled connection
+        that the lamp abandons and resumes via Range does not get a fresh budget just
+        because it is a new TCP connection. Only called when stall_after is set.
+        """
+        with self.lock:
+            sent_so_far = self.stall_sent.get(ip, 0)
+            allowed = max(0, min(n, self.stall_after - sent_so_far))
+            self.stall_sent[ip] = sent_so_far + allowed
+            return allowed
+
     def record(self, ip: str, target: str, version: str, nbytes: int, status: str) -> None:
         with self.lock:
             if status == "ok":
@@ -175,16 +194,22 @@ class Handler(BaseHTTPRequestHandler):
         sent = 0
         status = "ok"
         remaining = size - start
+        client_ip = self.client_address[0]
         try:
             with path.open("rb") as f:
                 f.seek(start)
                 while sent < remaining:
-                    if self.station.stall_after is not None and sent >= self.station.stall_after:
+                    read_size = min(CHUNK, remaining - sent)
-                        # test aid: hold the connection open and send nothing more, so the
+                    if self.station.stall_after is not None:
-                        # firmware's stall handling can be exercised on real hardware. We
+                        # Reserve against this client's cumulative budget, not just this
-                        # still notice if the peer gives up and closes its side, so a test
+                        # request's byte count - see Station.reserve_stall_bytes.
-                        # (or an impatient client) that hangs up is recorded as "aborted"
+                        read_size = self.station.reserve_stall_bytes(client_ip, read_size)
-                        # instead of leaking the thread forever.
+                        if read_size == 0:
+                            # test aid: hold the connection open and send nothing more, so
+                            # the firmware's stall handling can be exercised on real
+                            # hardware. We still notice if the peer gives up and closes its
+                            # side, so a test (or an impatient client) that hangs up is
+                            # recorded as "aborted" instead of leaking the thread forever.
                             while True:
                                 readable, _, _ = select.select([self.connection], [], [], 1.0)
                                 if readable:
@@ -194,12 +219,6 @@ class Handler(BaseHTTPRequestHandler):
                                         peek = b""
                                     if not peek:
                                         raise ConnectionResetError("peer closed during stall")
-                    read_size = CHUNK
-                    if self.station.stall_after is not None:
-                        # Don't overshoot the stall boundary within a single read: without
-                        # this a whole CHUNK could be sent before the check above ever
-                        # notices we've passed --stall-after.
-                        read_size = min(read_size, max(1, self.station.stall_after - sent))
                     chunk = f.read(read_size)
                     if not chunk:
                         break

tools/station/tests/test_server.py

@@ -278,6 +278,62 @@ def test_stall_after_stops_sending_but_keeps_the_connection(tmp_path):
         s.stop()
 
 
+def test_stall_after_budget_survives_a_resumed_request(tmp_path):
+    """--stall-after must stop a client for good, not just for one request.
+
+    On real hardware, a lamp whose download stalls hits its own receive timeout, gives up
+    on the connection, and opens a new one with a Range header picking up right where it
+    left off - exactly what test_stall_after_stops_sending_but_keeps_the_connection above
+    exercises for a single request. A per-request byte counter would hand that resumed
+    request a brand-new --stall-after allowance, so the "stall" would only ever delay
+    delivery instead of actually stopping it - which is what was observed on the EVK: the
+    server stalled at 65536 bytes each time, but the lamp reconnected five times and walked
+    away with the complete image. This test reproduces that reconnect and checks that the
+    second, resumed request gets no further bytes at all.
+    """
+    s = Station(tmp_path, "--stall-after", "1000")
+    try:
+        make_image(s.image_dir / "ninlight_evk_ota.img", "evk", "1.0.0", b"y" * 200000)
+
+        # First request: burns through the whole --stall-after budget, then the server
+        # holds the connection open and sends nothing further.
+        sock = socket.create_connection(("127.0.0.1", s.port), timeout=5)
+        sock.sendall(b"GET /ninlight_evk_ota.img HTTP/1.1\r\nHost: x\r\n\r\n")
+        got = b""
+        sock.settimeout(2)
+        try:
+            while len(got) < 200000:
+                chunk = sock.recv(65536)
+                if not chunk:
+                    break
+                got += chunk
+        except socket.timeout:
+            pass
+        assert len(got) < 20000  # far less than the full image, same bound as above
+        sock.close()  # the lamp's watchdog gives up and reconnects, like on real hardware
+
+        # Second request, resuming with Range exactly like a real lamp would. This
+        # client's cumulative budget is already spent, so it must get zero more bytes of
+        # body - not a fresh --stall-after allowance just because it's a new connection.
+        sock2 = socket.create_connection(("127.0.0.1", s.port), timeout=5)
+        sock2.sendall(b"GET /ninlight_evk_ota.img HTTP/1.1\r\nHost: x\r\nRange: bytes=1000-\r\n\r\n")
+        sock2.settimeout(2)
+        header = b""
+        while b"\r\n\r\n" not in header:
+            header += sock2.recv(4096)
+        assert b" 206 " in header.split(b"\r\n", 1)[0]
+        body_after_header = header.split(b"\r\n\r\n", 1)[1]
+        assert body_after_header == b"", "resumed request must not receive any body bytes"
+        try:
+            more = sock2.recv(1)
+        except socket.timeout:
+            more = None
+        assert more in (None, b""), "server sent data after the client's stall budget was spent"
+        sock2.close()
+    finally:
+        s.stop()
+
+
 def test_concurrent_downloads_are_all_recorded(station):
     """Several lamps hitting the station at once must not lose or corrupt status.json.