fix: reject non-object JSON bodies on /predict endpoint (#819)#832
Open
Rudra-clrscr wants to merge 5 commits into
Open
fix: reject non-object JSON bodies on /predict endpoint (#819)#832Rudra-clrscr wants to merge 5 commits into
Rudra-clrscr wants to merge 5 commits into
Conversation
…4#819) The /predict route already validated missing/empty text, wrong types, and max length, but request.get_json(silent=True) or {} assumed a dict — a valid JSON body that wasn't an object (list, string, number) caused data.get("text") to raise AttributeError, which the catch-all handler turned into a 500 leaking the exception message. Malformed JSON bodies fell through to a misleading "No text provided" message. Now both cases return a clear 400 with a consistent {"error": ...} payload, matching the existing test_predict_input_validation.py expectations.
|
Someone is attempting to deploy a commit to the Aditya Sharma's projects Team on Vercel. A member of the Team first needs to authorize it. |
There was a problem hiding this comment.
Pull request overview
This PR hardens the Flask ML API’s /predict endpoint input validation to prevent 500s when the request body is valid JSON but not an object, and to return clearer 400-level errors for invalid JSON bodies.
Changes:
- Adds explicit type-checking for the parsed JSON body in
/predictto require a JSON object (dict). - Returns 400 for malformed/non-JSON bodies instead of allowing the request to fall through to later validation and/or exceptions.
- Returns 400 for non-dict JSON bodies (e.g., list/string/number) with an explanatory error message.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
added 2 commits
July 14, 2026 04:38
The container's Flask process defaults to FLASK_HOST=127.0.0.1 (a deliberate secure default for bare-metal runs), so inside the CI container it never listens on an interface reachable through the -p 5000:5000 port mapping, and the smoke test's curl always times out. docker-compose.yml already sets FLASK_HOST=0.0.0.0 for the same reason; the CI workflow's docker run step needs the same override.
main's copies of these files have a duplicate 'function App()' declaration and a stray '#Manipulation' line before an import respectively - both invalid JS that break 'vite build' in the frontend Docker image stage. Only fixed inside the open fix/frontend-eslint-errors PR branch, never merged to main, so any branch cut from main (including this one) inherits them. Pulling in the already-verified fix.
| } | ||
| }; | ||
|
|
||
| // Helper to get earned badges (returns array of badge objects) |
| } finally { | ||
| setLoading(false); | ||
|
|
||
| function App() { |
| const [hasCelebrated, setHasCelebrated] = useState(() => { | ||
| return localStorage.getItem("firstPrediction") === "true"; | ||
| }); | ||
| const [showCelebration, setShowCelebration] = useState(false); |
Owner
|
@Rudra-clrscr some code is removed from app.jsx as the other pr also link if i merge one then code base reflect fix this after i merge all. |
added 2 commits
July 14, 2026 18:57
App.jsx and ManipulationIndex.jsx changes pulled in a Docker-CI-only fix from an unrelated PR and made this PR's diff show large, unrelated deletions (dead badge/celebration state cleanup that belongs to the frontend-eslint-errors PR, not this one). Reverting so review stays focused on the actual /predict input-validation fix. The Docker frontend-build check on this PR will fail again until the frontend-eslint-errors (or automated-test-suite) PR merges first - expected, since main itself still carries the parse bugs those PRs fix.
… payloads get_json(silent=True) returns None both for malformed JSON and for the valid JSON literal 'null', so the previous code told them apart only by whether the raw body was non-empty and then reported both cases as 'must be a valid JSON object' - misleading for a well-formed 'null' body. Now a genuinely malformed body is distinguished by attempting a real json.loads() parse, and valid non-object bodies are covered by new parametrized tests (list/string/number/null).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes #819.
Most of the requested validation on
/predict(missing text, empty/whitespacetext, non-string type, max length, consistent
{"error": ...}format) wasalready implemented. The remaining gap:
data = request.get_json(silent=True) or {}assumed the parsed JSON body was always a dict. A valid-but-non-object body
(e.g.
[1,2,3],"hello",42) madedata.get("text")raiseAttributeError,which the catch-all handler turned into a 500 that leaked the raw exception
string. Malformed JSON also produced a misleading "No text provided" message
instead of flagging the bad body.
Changes
/predict"No text provided"
Test plan
backend/tests/test_predict_input_validation.py::test_non_object_body_rejectedwas already written for this and failing before the fix; passes now
pytest backend/tests/) — no regressionsfrom this change (102 tests pass; 6 pre-existing failures unrelated to this
issue, caused by an undefined
internal_endpoint_requiredname and otherunrelated bugs already present on
main)