mirror of
https://github.com/ipnet-mesh/meshcore-hub.git
synced 2026-08-11 11:22:50 +02:00
fix(web): forward repeated query params through the API proxy
The SPA reaches the backend via the web api_proxy, which forwarded query params using dict(request.query_params). dict() on Starlette's QueryParams multidict keeps only the last value of a repeated key, so a multi-valued filter like ?observed_by=A&observed_by=B was forwarded to the backend as observed_by=B only. The backend then filtered to B's events, making a message observed only by A disappear as soon as B was also selected — the reported "filters act like AND" symptom. This happens independently of the Redis response cache. Forward request.query_params.multi_items() (a list of (key, value) tuples) so all repeated values reach the backend. Add web proxy regression tests asserting both observed_by values are forwarded, and capture forwarded params in the MockHttpClient. This is the primary fix; the earlier cache-key change (multi_items in sorted_query_string) addressed the same collapse pattern at the cache layer. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
@@ -18,7 +18,22 @@ Investigation findings:
|
||||
- The frontend correctly sends repeated params: `?observed_by=A&observed_by=B`
|
||||
(`web/static/js/spa/api.js` appends each array item; `pages/messages.js:263`).
|
||||
|
||||
**Root cause — cache key collision.** The cached-response key is built by
|
||||
**Primary root cause — the web proxy collapses repeated params.** The SPA calls
|
||||
the backend through the web API proxy (`src/meshcore_hub/web/app.py:696`
|
||||
`api_proxy`). It forwarded query params with:
|
||||
|
||||
```python
|
||||
params = dict(request.query_params) # collapses repeated keys to LAST value
|
||||
```
|
||||
|
||||
`dict(QueryParams)` keeps only the last value of a repeated key, so
|
||||
`?observed_by=A&observed_by=B` was forwarded to the backend as `observed_by=B`
|
||||
only. The backend then filtered to B's events, dropping the A-only message — the
|
||||
exact reported symptom, and independent of caching. Fix: forward
|
||||
`request.query_params.multi_items()` (a list of `(key, value)` tuples; httpx
|
||||
preserves them as repeated params).
|
||||
|
||||
**Secondary root cause — cache key collision.** The cached-response key is built by
|
||||
`sorted_query_string` (`src/meshcore_hub/api/cache.py:15`):
|
||||
|
||||
```python
|
||||
|
||||
@@ -719,8 +719,11 @@ def create_app(
|
||||
client: httpx.AsyncClient = request.app.state.http_client
|
||||
url = f"/api/{path}"
|
||||
|
||||
# Forward query parameters
|
||||
params = dict(request.query_params)
|
||||
# Forward query parameters. Use multi_items() (a list of (key, value)
|
||||
# tuples) rather than dict(...), which would collapse repeated keys to
|
||||
# the last value and drop all but one value of multi-valued filters such
|
||||
# as observed_by (?observed_by=A&observed_by=B).
|
||||
params = request.query_params.multi_items()
|
||||
|
||||
# Forward body for write methods
|
||||
body = None
|
||||
|
||||
@@ -28,6 +28,9 @@ class MockHttpClient:
|
||||
def __init__(self) -> None:
|
||||
"""Initialize mock client with default responses."""
|
||||
self._responses: dict[str, dict[str, Any]] = {}
|
||||
# Records the params forwarded by the most recent request() call so
|
||||
# tests can assert how the proxy forwards query parameters.
|
||||
self.last_request_params: Any = None
|
||||
self._default_responses()
|
||||
|
||||
def _default_responses(self) -> None:
|
||||
@@ -265,6 +268,7 @@ class MockHttpClient:
|
||||
headers: dict | None = None,
|
||||
) -> Response:
|
||||
"""Mock generic request (used by API proxy)."""
|
||||
self.last_request_params = params
|
||||
key = f"{method.upper()}:{url}"
|
||||
if key in self._responses:
|
||||
return self._create_response(key)
|
||||
|
||||
@@ -175,6 +175,32 @@ class TestConfigJsonXssEscaping:
|
||||
assert parsed["role_names"]["test"] == "test"
|
||||
|
||||
|
||||
class TestApiProxyQueryParams:
|
||||
"""The proxy must forward repeated query params without collapsing them."""
|
||||
|
||||
def test_repeated_query_params_all_forwarded(
|
||||
self, client: TestClient, mock_http_client: MockHttpClient
|
||||
) -> None:
|
||||
# Multi-valued observer filter: the backend must receive BOTH values.
|
||||
# dict(request.query_params) would drop "A" and only forward "B",
|
||||
# making OR-filtered messages disappear when a second observer is added.
|
||||
client.get("/api/v1/messages?observed_by=A&observed_by=B")
|
||||
|
||||
forwarded = mock_http_client.last_request_params
|
||||
pairs = list(forwarded) # list of (key, value) tuples
|
||||
assert ("observed_by", "A") in pairs
|
||||
assert ("observed_by", "B") in pairs
|
||||
|
||||
def test_single_query_param_forwarded(
|
||||
self, client: TestClient, mock_http_client: MockHttpClient
|
||||
) -> None:
|
||||
client.get("/api/v1/messages?observed_by=A&limit=10")
|
||||
|
||||
pairs = list(mock_http_client.last_request_params)
|
||||
assert ("observed_by", "A") in pairs
|
||||
assert ("limit", "10") in pairs
|
||||
|
||||
|
||||
class TestCheckApiAccess:
|
||||
"""Unit tests for check_api_access with _OPEN, _AUTHENTICATED, and role-based levels."""
|
||||
|
||||
|
||||
Reference in New Issue
Block a user