fix(update): Lampe bekommt genau die geprüften Bytes
Die Registry für Einmal-URLs speicherte bisher nur einen Pfad; View und verify() lasen die Datei getrennt und zu unterschiedlichen Zeitpunkten erneut ein. Wurde die Datei zwischen Prüfung und Abruf ersetzt (z. B. gleicher Dateiname in build/ und build-nocreds/), erreichte ein nie geprüftes Image die Lampe und hebelte damit die WLAN-Sicherung aus. Jetzt werden die Bytes einmal im Install-Pfad gelesen, Gerätetyp/Version und WLAN-SSID auf genau diesen Bytes geprüft (FirmwareCatalog.verify_and_read, firmware.contains_ssid) und exakt diese Bytes in der Registry hinterlegt; die View serviert nur noch das, was tatsächlich geprüft wurde. Außerdem: Bewertungen (Katalog-Rescan-Signal) werden nicht mehr angestoßen, während ein Update läuft - das würde eine gerade flashende Lampe zusätzlich anfragen und Angebot/Issue unter dem laufenden Update verändern; nach jedem Update wird stattdessen genau einmal neu bewertet. Kommentar zum Install-Lock korrigiert (der eigentliche Lücke liegt vor dem Setzen der Flags, nicht in HAs eigener "bereits läuft"-Prüfung). Testlücke geschlossen: die View liefert jetzt nachweislich nur die zu ihrem Schlüssel gehörenden Bytes. Timeout in einem WLAN-Regressionstest verkürzt, damit er bei einem Rückfall nicht 12 Minuten hängt.
This commit is contained in:
@@ -3,7 +3,6 @@
|
||||
from __future__ import annotations
|
||||
|
||||
from collections.abc import Mapping
|
||||
from pathlib import Path
|
||||
import secrets
|
||||
from typing import Any
|
||||
|
||||
@@ -17,24 +16,29 @@ from .const import CONF_LAMP_BASE_URL, DOMAIN, FIRMWARE_URL_PREFIX
|
||||
|
||||
|
||||
class OtaRegistry:
|
||||
"""Random keys, each bound to exactly one file, valid only while its update runs."""
|
||||
"""Random keys, each bound to exactly the verified bytes of one image, valid only while its
|
||||
update runs.
|
||||
|
||||
Bytes rather than a path: the file on disk could be replaced between verification and
|
||||
download (spec §7 guard would otherwise apply to a file the lamp never actually receives).
|
||||
"""
|
||||
|
||||
def __init__(self) -> None:
|
||||
self._paths: dict[str, Path] = {}
|
||||
self._images: dict[str, bytes] = {}
|
||||
|
||||
def create(self, path: Path) -> str:
|
||||
def create(self, data: bytes) -> str:
|
||||
key = secrets.token_urlsafe(32)
|
||||
self._paths[key] = path
|
||||
self._images[key] = data
|
||||
return key
|
||||
|
||||
def revoke(self, key: str) -> None:
|
||||
self._paths.pop(key, None)
|
||||
self._images.pop(key, None)
|
||||
|
||||
def get(self, key: str) -> Path | None:
|
||||
return self._paths.get(key)
|
||||
def get(self, key: str) -> bytes | None:
|
||||
return self._images.get(key)
|
||||
|
||||
def __len__(self) -> int:
|
||||
return len(self._paths)
|
||||
return len(self._images)
|
||||
|
||||
|
||||
class NinlightFirmwareView(HomeAssistantView):
|
||||
@@ -49,12 +53,8 @@ class NinlightFirmwareView(HomeAssistantView):
|
||||
self._registry = registry
|
||||
|
||||
async def get(self, request: web.Request, key: str) -> web.Response:
|
||||
path = self._registry.get(key)
|
||||
if path is None:
|
||||
return web.Response(status=404)
|
||||
try:
|
||||
body = await self._hass.async_add_executor_job(path.read_bytes)
|
||||
except OSError:
|
||||
body = self._registry.get(key)
|
||||
if body is None:
|
||||
return web.Response(status=404)
|
||||
return web.Response(body=body, content_type="application/octet-stream")
|
||||
|
||||
|
||||
@@ -28,6 +28,7 @@ from .const import (
|
||||
from .coordinator import NinlightConfigEntry, NinlightCoordinator
|
||||
from .entity import NinlightEntity
|
||||
from .firmware import FirmwareImage
|
||||
from .firmware import contains_ssid as _firmware_contains_ssid
|
||||
from .firmware import is_newer as _firmware_is_newer
|
||||
from .http import lamp_base_url
|
||||
from .shared import DATA_SHARED
|
||||
@@ -105,6 +106,11 @@ class NinlightUpdateEntity(NinlightEntity, UpdateEntity):
|
||||
|
||||
@callback
|
||||
def _schedule_evaluation(self) -> None:
|
||||
# Never evaluate while an install is running: it would poll a flashing lamp for settings
|
||||
# and could churn the offer/issue underneath the install. `_async_install_locked` always
|
||||
# schedules exactly one evaluation itself once the flags are cleared.
|
||||
if self._attr_in_progress:
|
||||
return
|
||||
self.coordinator.config_entry.async_create_task(
|
||||
self.hass, self._async_evaluate(), name="ninlight firmware evaluation"
|
||||
)
|
||||
@@ -170,9 +176,10 @@ class NinlightUpdateEntity(NinlightEntity, UpdateEntity):
|
||||
|
||||
async def async_install(self, version: str | None, backup: bool, **kwargs: Any) -> None:
|
||||
# Only the call that owns this lock may touch in_progress/ota_in_progress below: without
|
||||
# this check, a second concurrent call would sail through HA's own in-progress guard (it
|
||||
# only rejects installed == latest) and its `finally` would clear the flags out from under
|
||||
# the first call while that one is still flashing the lamp.
|
||||
# this check, a second concurrent call could reach the image verification and lamp
|
||||
# re-checks before those flags are set - HA's own "already in progress" guard only helps
|
||||
# once _attr_in_progress is already True - and its `finally` would clear the flags out
|
||||
# from under the first call while that one is still flashing the lamp.
|
||||
if self._install_lock.locked():
|
||||
raise _error("update_busy")
|
||||
async with self._install_lock:
|
||||
@@ -184,7 +191,11 @@ class NinlightUpdateEntity(NinlightEntity, UpdateEntity):
|
||||
if image is None:
|
||||
raise _error("no_update")
|
||||
base = lamp_base_url(self.hass, self.coordinator.config_entry.options)
|
||||
if not await self.hass.async_add_executor_job(shared.catalog.verify, image):
|
||||
# Read the bytes ONCE here and verify exactly those bytes: the view below serves this
|
||||
# same in-memory copy, so nothing that happens to the file afterwards (e.g. another build
|
||||
# overwriting it mid-update) can change what actually reaches the lamp.
|
||||
data = await self.hass.async_add_executor_job(shared.catalog.verify_and_read, image)
|
||||
if data is None:
|
||||
raise _error("image_invalid")
|
||||
|
||||
# Re-check right before sending: the lamp's firmware may have changed since the offer was
|
||||
@@ -198,11 +209,11 @@ class NinlightUpdateEntity(NinlightEntity, UpdateEntity):
|
||||
raise _error("image_invalid")
|
||||
ssid = _ssid(info)
|
||||
if depends_on_compiled_wifi(settings) and not await self.hass.async_add_executor_job(
|
||||
shared.catalog.contains_ssid, image, ssid
|
||||
_firmware_contains_ssid, data, ssid
|
||||
):
|
||||
raise _error("wifi_guard", ssid=ssid or "?")
|
||||
|
||||
key = shared.registry.create(image.path)
|
||||
key = shared.registry.create(data)
|
||||
url = firmware_url(base, key)
|
||||
if len(url) > MAX_LAMP_URL_LENGTH:
|
||||
shared.registry.revoke(key)
|
||||
@@ -220,6 +231,7 @@ class NinlightUpdateEntity(NinlightEntity, UpdateEntity):
|
||||
self.coordinator.ota_in_progress = False
|
||||
await self.coordinator.async_refresh()
|
||||
self.async_write_ha_state()
|
||||
self._schedule_evaluation()
|
||||
|
||||
async def _async_wait_for(self, target: str, start_uptime: Any) -> None:
|
||||
"""Wait for the lamp to report the target version, an error, or an old-version reboot.
|
||||
|
||||
Reference in New Issue
Block a user