Skip to content

fix(server): answer a malformed or non-object request body with 400 - #153

Open
kevin9327 wants to merge 1 commit into
Edge0-AI:mainfrom
kevin9327:fix/server-malformed-request-body
Open

kevin9327 wants to merge 1 commit into
Edge0-AI:mainfrom
kevin9327:fix/server-malformed-request-body

Conversation

@kevin9327

Copy link
Copy Markdown
Contributor

What & why

A request body that is not valid JSON, or is valid JSON but not a chat request, gets no usable answer:

  • stdlib transport: do_POST runs json.loads outside any try, so a malformed body ({not json, or bytes that are not UTF-8) raises inside the handler. The client sees the connection closed with no HTTP response, while the Flask transport and the other validation errors answer 400 invalid_request.
  • Both transports: parse_chat_request assumes the shape. [1, 2], "hello", {"messages": "hi"}, {"messages": ["hi"]} and {"messages": [null]} raise AttributeError from .get, which is a 500 on Flask and a dropped connection on stdlib.

This PR makes the stdlib handler answer a malformed body with 400 (request body is not valid JSON: ...), and makes parse_chat_request raise the same ValueError the other bad inputs raise (request body must be a JSON object, messages must be a list of message objects), so both transports answer 400 before any generation or SSE headers.

No issue filed; I found this while reading server/app.py.

Area

  • Python framework (python/)
  • macOS (macos/)
  • iOS (ios/)
  • Android (android/)
  • Windows (windows/)
  • Docs / other

How was it tested?

python/tests/test_server.py gets two parametrized tests: malformed bodies on /v1/chat/completions and /v1/completions over the stdlib transport, and five non-chat-request bodies over both transports. All 14 new cases fail on main (connection dropped or AttributeError) and pass with the change. Each also asserts the fake engine generated nothing.

Ran on Linux with mlx[cpu]==0.30.6 and mlx-lm==0.31.0: pytest -m 'not slow' tests gives 181 passed, 16 failed. The 16 failures are all in tests/test_streaming_math.py and fail identically on main (167 passed, same 16 failed) on this CPU build; I have no Apple silicon to run them on.

Checklist

  • pytest -m 'not slow' passes locally for the server tests (see above for the unrelated test_streaming_math failures on a CPU-only MLX)
  • Repo hygiene: no new MLX imports, no paths or secrets
  • Added/updated tests for new behavior
  • Did not bump mlx past the pinned 0.30.6
  • Updated relevant docs / READMEs: none needed (no user-facing option changes)

AI-assisted: I used an AI coding assistant for this change. I reviewed the code and ran the tests above.

Signed-off-by: kevin9327 <5299031+kevin9327@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant