mirror of
https://github.com/calibrain/shelfmark.git
synced 2026-10-01 08:04:58 +01:00
fix(requests): reject non-object items in the batch endpoint (#1369)
`POST /api/requests/batch` checks that `requests` is a non-empty list and then hands each element to the shared preparation helper, which calls `.get()` on it. A bare string, number or null in the list raises `AttributeError` and the caller gets a 500, while `POST /api/requests` answers 400 with a message for the same mistake. This validates the element type beside the existing list check. One bad item rejects the whole batch rather than being reported per item, which matches the endpoint's current contract: every other failure path already aborts the batch with a single error body, as `test_batch_create_requests_is_atomic` asserts. Responses for valid payloads are unchanged. ## Verification - `tests/core/test_request_routes_api.py`: a case per bad shape (int, str, null, list), plus a mixed valid and invalid batch that asserts nothing was created. All fail on current main with a 500 and pass here. - Full suite (3168), ruff, ruff format, basedpyright, vulture green.
This commit is contained in:
@@ -715,6 +715,8 @@ def register_request_routes(
|
||||
raw_requests = data.get("requests")
|
||||
if not isinstance(raw_requests, list) or len(raw_requests) == 0:
|
||||
return jsonify({"error": "requests must contain at least one request"}), 400
|
||||
if not all(isinstance(raw_request, dict) for raw_request in raw_requests):
|
||||
return jsonify({"error": "requests must contain objects"}), 400
|
||||
|
||||
try:
|
||||
prepared_requests = [
|
||||
|
||||
@@ -775,6 +775,54 @@ class TestRequestRoutes:
|
||||
assert resp.json["code"] == "duplicate_pending_request"
|
||||
assert main_module.user_db.list_requests(user_id=user["id"]) == []
|
||||
|
||||
@pytest.mark.parametrize("bad_item", [1, "book", None, ["x"]])
|
||||
def test_batch_rejects_non_object_request_items(self, main_module, client, bad_item):
|
||||
user = _create_user(main_module, prefix="reader")
|
||||
_set_session(client, user_id=user["username"], db_user_id=user["id"], is_admin=False)
|
||||
|
||||
with patch.object(main_module, "get_auth_mode", return_value="builtin"):
|
||||
resp = client.post("/api/requests/batch", json={"requests": [bad_item]})
|
||||
|
||||
assert resp.status_code == 400
|
||||
assert "requests must contain objects" in resp.json["error"]
|
||||
|
||||
def test_batch_non_object_item_rejects_whole_batch(self, main_module, client):
|
||||
user = _create_user(main_module, prefix="reader")
|
||||
_set_session(client, user_id=user["username"], db_user_id=user["id"], is_admin=False)
|
||||
policy = _policy(default_ebook="request_book")
|
||||
|
||||
valid_request = {
|
||||
"book_data": {
|
||||
"title": "Batch Valid Alongside Bad Item",
|
||||
"author": "Shelfmark",
|
||||
"content_type": "ebook",
|
||||
"provider": "openlibrary",
|
||||
"provider_id": "batch-valid-alongside-bad-1",
|
||||
},
|
||||
"context": {
|
||||
"source": "*",
|
||||
"content_type": "ebook",
|
||||
"request_level": "book",
|
||||
},
|
||||
}
|
||||
|
||||
with patch.object(main_module, "get_auth_mode", return_value="builtin"):
|
||||
with patch.object(
|
||||
main_module, "load_users_request_policy_settings", return_value=policy
|
||||
):
|
||||
with patch(
|
||||
"shelfmark.core.request_routes.load_users_request_policy_settings",
|
||||
return_value=policy,
|
||||
):
|
||||
resp = client.post(
|
||||
"/api/requests/batch",
|
||||
json={"requests": [valid_request, "not-an-object"]},
|
||||
)
|
||||
|
||||
assert resp.status_code == 400
|
||||
assert "requests must contain objects" in resp.json["error"]
|
||||
assert main_module.user_db.list_requests(user_id=user["id"]) == []
|
||||
|
||||
def test_create_request_emits_websocket_events(self, main_module, client):
|
||||
user = _create_user(main_module, prefix="reader")
|
||||
_set_session(client, user_id=user["username"], db_user_id=user["id"], is_admin=False)
|
||||
|
||||
Reference in New Issue
Block a user