fix(api): normalize non-object JSON bodies to empty dict in token PATCH#3976
Conversation
0139df5 to
0fd29a7
Compare
alteixeira20
left a comment
There was a problem hiding this comment.
Thanks for the small targeted fix. The implementation direction looks correct, but I would like this to include focused regression coverage before approval.
The linked issue asks specifically for coverage of valid non-object JSON bodies reaching the PATCH route, especially [] and null. Right now the PR only changes routes/api_token_routes.py, so the behavior is fixed in code but not protected against regression.
Please add focused tests, preferably in tests/test_api_token_routes.py, covering:
PATCH /api/tokens/{token_id}with body[]does not raise a 500;PATCH /api/tokens/{token_id}with bodynulldoes not raise a 500;- the normal object body path still behaves as before.
After that I can re-run the focused audit and merge if everything stays green.
Valid non-dict JSON (e.g. [] or null) reaches payload.get(...) and
raises AttributeError. Normalize to {} so the route returns a controlled
response instead of an unhandled 500.
Fixes odysseus-dev#3966
Covers array body ([]), null body, and normal object body as requested in alteixeira20's review of odysseus-dev#3976.
9b0f22a to
15f5b66
Compare
|
All three regression tests are pushed and CI is green. Ready for re-review when you get a chance. |
alteixeira20
left a comment
There was a problem hiding this comment.
LGTM. Re-audited the current head after the requested regression coverage was added. The fix is narrowly scoped to normalizing valid non-object JSON payloads on the API token PATCH route, and the added tests cover [], null, and the normal object update path. The merge probe is clean, GitHub checks are green, changed-module compile passed, and git diff --check passed. The local test_security_regressions.py failures are unrelated to this PR and caused by a missing local nh3 dependency.
…CH (odysseus-dev#3976) * fix(api): normalize non-object JSON bodies to empty dict in token PATCH Valid non-dict JSON (e.g. [] or null) reaches payload.get(...) and raises AttributeError. Normalize to {} so the route returns a controlled response instead of an unhandled 500. Fixes odysseus-dev#3966 * test(api): add regression tests for PATCH with non-object JSON bodies Covers array body ([]), null body, and normal object body as requested in alteixeira20's review of odysseus-dev#3976. --------- Co-authored-by: michaelxer <michaelxer@users.noreply.github.com>
…CH (odysseus-dev#3976) * fix(api): normalize non-object JSON bodies to empty dict in token PATCH Valid non-dict JSON (e.g. [] or null) reaches payload.get(...) and raises AttributeError. Normalize to {} so the route returns a controlled response instead of an unhandled 500. Fixes odysseus-dev#3966 * test(api): add regression tests for PATCH with non-object JSON bodies Covers array body ([]), null body, and normal object body as requested in alteixeira20's review of odysseus-dev#3976. --------- Co-authored-by: michaelxer <michaelxer@users.noreply.github.com>
…CH (odysseus-dev#3976) * fix(api): normalize non-object JSON bodies to empty dict in token PATCH Valid non-dict JSON (e.g. [] or null) reaches payload.get(...) and raises AttributeError. Normalize to {} so the route returns a controlled response instead of an unhandled 500. Fixes odysseus-dev#3966 * test(api): add regression tests for PATCH with non-object JSON bodies Covers array body ([]), null body, and normal object body as requested in alteixeira20's review of odysseus-dev#3976. --------- Co-authored-by: michaelxer <michaelxer@users.noreply.github.com>
…CH (odysseus-dev#3976) * fix(api): normalize non-object JSON bodies to empty dict in token PATCH Valid non-dict JSON (e.g. [] or null) reaches payload.get(...) and raises AttributeError. Normalize to {} so the route returns a controlled response instead of an unhandled 500. Fixes odysseus-dev#3966 * test(api): add regression tests for PATCH with non-object JSON bodies Covers array body ([]), null body, and normal object body as requested in alteixeira20's review of odysseus-dev#3976. --------- Co-authored-by: michaelxer <michaelxer@users.noreply.github.com>
Summary
The
/api/tokens/{token_id}PATCH route catches JSON parse failures and falls back to{}, but after a successful parse it assumes the payload is a mapping. Valid non-dict JSON (e.g.[]ornull) reachespayload.get(...)and raisesAttributeError, returning an unhandled 500 to the client.Add a type check after parsing: if the payload is not a dict, normalize it to
{}so the route treats it as an empty update body.Target branch
dev, notmain. All PRs land indev;mainis curated by the maintainer at each release. If your PR is onmainby accident, click "Edit" on this PR and change the base.Linked Issue
Fixes #3966
Type of Change
Checklist
devHow to Test
devand authenticate as admin.curl -X PATCH http://localhost:PORT/api/tokens/TOKEN_ID -H "Content-Type: application/json" -d "[]"— should return the token unchanged (currently returns 500).-d "null"and-d "\"hello\""— same result.-d '{"name":"test"}'should still work as before.