Skip to content

fix(#28): reject unknown fields in POST and PATCH /items endpoints #29

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
fullsend-ai-coder wants to merge 1 commit into
base: main
Choose a base branch
from
+45 −0
Open

fix(#28): reject unknown fields in POST and PATCH /items endpoints #29

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions app.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,11 @@ def create_item():
if not data or "name" not in data:
return jsonify(error="name is required"), 400

allowed = {"name"}

@fullsend-ai-review fullsend-ai-review Bot Aug 21, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[medium] scope-creep

POST /items validation logic was added but is not authorized by issue #28. The issue exclusively describes the PATCH /items/:id bug. While POST exhibits the same silent-drop behavior, the fix extends beyond the linked issue's scope.

Suggested fix: Update issue #28 to include POST /items, or split the POST changes into a separate issue and PR.

unknown = set(data.keys()) - allowed
if unknown:
return jsonify(error=f"Unknown fields: {', '.join(sorted(unknown))}"), 400

@fullsend-ai-review fullsend-ai-review Bot Aug 21, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[low] error-message-format

Error messages at lines 32 and 54 begin with uppercase 'Unknown fields' but the established codebase pattern uses lowercase (e.g., 'name is required', 'item not found').

Suggested fix: Change both to lowercase: error=f"unknown fields: {', '.join(sorted(unknown))}"


item = {
"id": len(_get_items()) + 1,
"name": data["name"],
Expand All @@ -43,6 +48,11 @@ def update_item(item_id):
return jsonify(error="item not found"), 404

data = request.get_json()
allowed = {"name", "done"}
unknown = set(data.keys()) - allowed

@fullsend-ai-review fullsend-ai-review Bot Aug 21, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[medium] nil/null handling

In update_item, request.get_json() can return None (empty body, wrong content type). The new validation at line 52 calls set(data.keys()) without a null guard, causing an unhandled AttributeError and HTTP 500. Unlike create_item, which guards with 'if not data or "name" not in data:' before the field validation block, update_item has no equivalent check.

Suggested fix: Add 'if not data: return jsonify(error="request body is required"), 400' after 'data = request.get_json()' and before the 'allowed' set definition.

if unknown:
return jsonify(error=f"Unknown fields: {', '.join(sorted(unknown))}"), 400

if "done" in data:
item["done"] = data["done"]
if "name" in data:
Expand Down
35 changes: 35 additions & 0 deletions test_app.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,3 +63,38 @@ def test_delete_item(client):
def test_delete_item_not_found(client):
resp = client.delete("/items/999")
assert resp.status_code == 404


def test_patch_unknown_fields_rejected(client):
client.post("/items", json={"name": "widget"})
resp = client.patch("/items/1", json={"warehouse_bin": "A-12"})
assert resp.status_code == 400
assert "warehouse_bin" in resp.get_json()["error"]


def test_patch_mixed_known_and_unknown_fields_rejected(client):
client.post("/items", json={"name": "widget"})
resp = client.patch("/items/1", json={"done": True, "warehouse_bin": "A-12"})
assert resp.status_code == 400
assert "warehouse_bin" in resp.get_json()["error"]


def test_patch_valid_fields_accepted(client):
client.post("/items", json={"name": "widget"})
resp = client.patch("/items/1", json={"name": "gadget", "done": True})
assert resp.status_code == 200
data = resp.get_json()
assert data["name"] == "gadget"
assert data["done"] is True


def test_create_item_unknown_fields_rejected(client):
resp = client.post("/items", json={"name": "widget", "warehouse_bin": "A-12"})
assert resp.status_code == 400
assert "warehouse_bin" in resp.get_json()["error"]


def test_create_item_valid_fields_accepted(client):
resp = client.post("/items", json={"name": "widget"})
assert resp.status_code == 201
assert resp.get_json()["name"] == "widget"
Loading