From e9eb029ae343a89ae41d9836e9bd59b3b830b067 Mon Sep 17 00:00:00 2001 From: Shaun Walsh Date: Fri, 26 Jun 2026 13:03:34 +0100 Subject: [PATCH 1/4] [BugFix] Add modalities and logprobs validation for /v1/chat/completions ChatCompletionRequest is imported from upstream vllm and cannot have Pydantic validators added directly. Validate at the handler level by checking the raw JSON body before dispatch: - Reject modalities containing non-string elements (e.g. [123]) - Reject logprobs with non-boolean values (Pydantic v2 coerces "yes" to True instead of returning 400) Unskip modalities_list_bad_element and logprobs_wrong_type tests. Partial fix for #3649. Co-Authored-By: Claude Opus 4.6 Signed-off-by: Shaun Walsh --- .../invalid_param_test/test_invalid_omni_chat.py | 8 ++------ vllm_omni/entrypoints/openai/api_server.py | 14 ++++++++++++++ 2 files changed, 16 insertions(+), 6 deletions(-) diff --git a/tests/dfx/reliability/invalid_param_test/test_invalid_omni_chat.py b/tests/dfx/reliability/invalid_param_test/test_invalid_omni_chat.py index 9e39b059b2a..7d5dce37483 100644 --- a/tests/dfx/reliability/invalid_param_test/test_invalid_omni_chat.py +++ b/tests/dfx/reliability/invalid_param_test/test_invalid_omni_chat.py @@ -15,9 +15,6 @@ pytestmark = [pytest.mark.slow, pytest.mark.omni] -_SKIP_ISSUE_3649 = pytest.mark.skip(reason="https://github.com/vllm-project/vllm-omni/issues/3649") - - def _minimal_chat_json(omni_server: OmniServer) -> dict[str, object]: """Minimal valid chat body; individual tests override one offending field.""" return { @@ -112,9 +109,8 @@ def _chat_completions_request_without_expectations(omni_server: OmniServer, case pytest.param( "modalities_list_bad", 400, - ("modalities", "value_error", ""), + ("modalities", "list of strings"), id="modalities_list_bad_element", - marks=_SKIP_ISSUE_3649, ), pytest.param( "response_format_json_schema_incomplete", @@ -122,7 +118,7 @@ def _chat_completions_request_without_expectations(omni_server: OmniServer, case ("response_format", "value_error", "json_schema"), id="invalid_response_format_json_schema", ), - pytest.param("logprobs_wrong_type", 400, "logprobs", id="logprobs_wrong_type", marks=_SKIP_ISSUE_3649), + pytest.param("logprobs_wrong_type", 400, ("logprobs", "boolean"), id="logprobs_wrong_type"), pytest.param( "logprobs_top_without_enabled", 400, diff --git a/vllm_omni/entrypoints/openai/api_server.py b/vllm_omni/entrypoints/openai/api_server.py index 2c6a3d87a98..da761e7be9a 100755 --- a/vllm_omni/entrypoints/openai/api_server.py +++ b/vllm_omni/entrypoints/openai/api_server.py @@ -1168,6 +1168,20 @@ def OmniAudioGenerate(request: Request) -> OmniOpenAIServingAudioGenerate | None @with_cancellation @load_aware_call async def create_chat_completion(request: ChatCompletionRequest, raw_request: Request): + raw_body = await raw_request.json() + if "modalities" in raw_body: + modalities = raw_body["modalities"] + if not isinstance(modalities, list) or not all(isinstance(m, str) for m in modalities): + raise HTTPException( + status_code=HTTPStatus.BAD_REQUEST.value, + detail="modalities must be a list of strings, e.g. " + '[\"text\", \"audio\", \"image\"]', + ) + if "logprobs" in raw_body and not isinstance(raw_body["logprobs"], bool): + raise HTTPException( + status_code=HTTPStatus.BAD_REQUEST.value, + detail="logprobs must be a boolean (true or false)", + ) metrics_header_format = raw_request.headers.get(ENDPOINT_LOAD_METRICS_FORMAT_HEADER_LABEL, "") handler = Omnichat(raw_request) if handler is None: From 981dd21418c7d2bf7f7c4d026ff359d5b033c025 Mon Sep 17 00:00:00 2001 From: Shaun Walsh Date: Tue, 21 Jul 2026 14:51:29 +0100 Subject: [PATCH 2/4] style: fix ruff format issues (blank line + string quoting) Signed-off-by: Shaun Walsh --- .../reliability/invalid_param_test/test_invalid_omni_chat.py | 1 + vllm_omni/entrypoints/openai/api_server.py | 3 +-- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/dfx/reliability/invalid_param_test/test_invalid_omni_chat.py b/tests/dfx/reliability/invalid_param_test/test_invalid_omni_chat.py index 7d5dce37483..6cd4ef4ff43 100644 --- a/tests/dfx/reliability/invalid_param_test/test_invalid_omni_chat.py +++ b/tests/dfx/reliability/invalid_param_test/test_invalid_omni_chat.py @@ -15,6 +15,7 @@ pytestmark = [pytest.mark.slow, pytest.mark.omni] + def _minimal_chat_json(omni_server: OmniServer) -> dict[str, object]: """Minimal valid chat body; individual tests override one offending field.""" return { diff --git a/vllm_omni/entrypoints/openai/api_server.py b/vllm_omni/entrypoints/openai/api_server.py index da761e7be9a..07241988a21 100755 --- a/vllm_omni/entrypoints/openai/api_server.py +++ b/vllm_omni/entrypoints/openai/api_server.py @@ -1174,8 +1174,7 @@ async def create_chat_completion(request: ChatCompletionRequest, raw_request: Re if not isinstance(modalities, list) or not all(isinstance(m, str) for m in modalities): raise HTTPException( status_code=HTTPStatus.BAD_REQUEST.value, - detail="modalities must be a list of strings, e.g. " - '[\"text\", \"audio\", \"image\"]', + detail='modalities must be a list of strings, e.g. ["text", "audio", "image"]', ) if "logprobs" in raw_body and not isinstance(raw_body["logprobs"], bool): raise HTTPException( From ca02d69750a30944f1d54efe36b5bec6f3ddb0c4 Mon Sep 17 00:00:00 2001 From: Shaun Walsh Date: Mon, 21 Sep 2026 11:47:53 +0100 Subject: [PATCH 3/4] Preserve null chat completion defaults Signed-off-by: Shaun Walsh --- vllm_omni/entrypoints/openai/api_server.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/vllm_omni/entrypoints/openai/api_server.py b/vllm_omni/entrypoints/openai/api_server.py index 28b63d463da..485828f08b6 100644 --- a/vllm_omni/entrypoints/openai/api_server.py +++ b/vllm_omni/entrypoints/openai/api_server.py @@ -1350,14 +1350,14 @@ def OmniAudioGenerate(request: Request) -> OmniOpenAIServingAudioGenerate | None @load_aware_call async def create_chat_completion(request: ChatCompletionRequest, raw_request: Request): raw_body = await raw_request.json() - if "modalities" in raw_body: + if "modalities" in raw_body and raw_body["modalities"] is not None: modalities = raw_body["modalities"] if not isinstance(modalities, list) or not all(isinstance(m, str) for m in modalities): raise HTTPException( status_code=HTTPStatus.BAD_REQUEST.value, detail='modalities must be a list of strings, e.g. ["text", "audio", "image"]', ) - if "logprobs" in raw_body and not isinstance(raw_body["logprobs"], bool): + if "logprobs" in raw_body and raw_body["logprobs"] is not None and not isinstance(raw_body["logprobs"], bool): raise HTTPException( status_code=HTTPStatus.BAD_REQUEST.value, detail="logprobs must be a boolean (true or false)", From 11adeb1d3772e895cacadded84e2e6c00ccd61da Mon Sep 17 00:00:00 2001 From: Shaun Walsh Date: Mon, 21 Sep 2026 11:51:34 +0100 Subject: [PATCH 4/4] Complete chat validation review fixes Signed-off-by: Shaun Walsh --- .../openai_api/test_api_server_guards.py | 40 +++++++++++++++++++ vllm_omni/entrypoints/openai/api_server.py | 31 ++++++++------ 2 files changed, 59 insertions(+), 12 deletions(-) diff --git a/tests/entrypoints/openai_api/test_api_server_guards.py b/tests/entrypoints/openai_api/test_api_server_guards.py index 71f93f6dccb..ab27f040034 100644 --- a/tests/entrypoints/openai_api/test_api_server_guards.py +++ b/tests/entrypoints/openai_api/test_api_server_guards.py @@ -698,6 +698,46 @@ def test_speech_without_handler_preserves_not_found_http_error() -> None: assert exc_info.value.detail == "The model does not support Speech API" +@pytest.mark.parametrize( + ("field", "value", "detail"), + [ + ("modalities", [123], "modalities must be a list of strings"), + ("logprobs", "yes", "logprobs must be a boolean"), + ], +) +def test_chat_completion_raw_body_guards_reject_lax_types(field, value, detail) -> None: + """The HTTP boundary rejects values that upstream Pydantic would coerce.""" + app = FastAPI() + app.state.openai_serving_chat = None + app.state.serving_tokenization = None + app.add_api_route("/v1/chat/completions", api_server.create_chat_completion, methods=["POST"]) + client = TestClient(app) + + payload = { + "model": "demo-model", + "messages": [{"role": "user", "content": "hello"}], + "stream": False, + field: value, + } + response = client.post("/v1/chat/completions", json=payload) + + assert response.status_code == 400 + assert detail in response.json()["detail"] + + +@pytest.mark.parametrize( + "raw_body", + [ + {"modalities": None}, + {"logprobs": None}, + {"modalities": None, "logprobs": None}, + ], +) +def test_chat_completion_raw_body_guards_allow_null_defaults(raw_body) -> None: + """Explicit JSON null keeps the upstream request model's default behavior.""" + api_server._validate_chat_completion_raw_body(raw_body) + + def test_engine_error_json_response_includes_request_and_stage_fields(monkeypatch) -> None: """Lock shared ``EngineGenerateError`` JSON fields used by Omni handlers. diff --git a/vllm_omni/entrypoints/openai/api_server.py b/vllm_omni/entrypoints/openai/api_server.py index 485828f08b6..def68df9858 100644 --- a/vllm_omni/entrypoints/openai/api_server.py +++ b/vllm_omni/entrypoints/openai/api_server.py @@ -1336,6 +1336,22 @@ def OmniAudioGenerate(request: Request) -> OmniOpenAIServingAudioGenerate | None return getattr(request.app.state, "openai_serving_audio_generate", None) +def _validate_chat_completion_raw_body(raw_body: dict[str, Any]) -> None: + """Reject values that upstream ChatCompletionRequest coerces too broadly.""" + if "modalities" in raw_body and raw_body["modalities"] is not None: + modalities = raw_body["modalities"] + if not isinstance(modalities, list) or not all(isinstance(m, str) for m in modalities): + raise HTTPException( + status_code=HTTPStatus.BAD_REQUEST.value, + detail='modalities must be a list of strings, e.g. ["text", "audio", "image"]', + ) + if "logprobs" in raw_body and raw_body["logprobs"] is not None and not isinstance(raw_body["logprobs"], bool): + raise HTTPException( + status_code=HTTPStatus.BAD_REQUEST.value, + detail="logprobs must be a boolean (true or false)", + ) + + @router.post( "/v1/chat/completions", dependencies=[Depends(validate_json_request)], @@ -1350,18 +1366,7 @@ def OmniAudioGenerate(request: Request) -> OmniOpenAIServingAudioGenerate | None @load_aware_call async def create_chat_completion(request: ChatCompletionRequest, raw_request: Request): raw_body = await raw_request.json() - if "modalities" in raw_body and raw_body["modalities"] is not None: - modalities = raw_body["modalities"] - if not isinstance(modalities, list) or not all(isinstance(m, str) for m in modalities): - raise HTTPException( - status_code=HTTPStatus.BAD_REQUEST.value, - detail='modalities must be a list of strings, e.g. ["text", "audio", "image"]', - ) - if "logprobs" in raw_body and raw_body["logprobs"] is not None and not isinstance(raw_body["logprobs"], bool): - raise HTTPException( - status_code=HTTPStatus.BAD_REQUEST.value, - detail="logprobs must be a boolean (true or false)", - ) + _validate_chat_completion_raw_body(raw_body) metrics_header_format = raw_request.headers.get(ENDPOINT_LOAD_METRICS_FORMAT_HEADER_LABEL, "") handler = Omnichat(raw_request) if handler is None: @@ -1438,6 +1443,8 @@ async def create_chat_completion(request: ChatCompletionRequest, raw_request: Re @with_cancellation @load_aware_call async def create_batch_chat_completion(request: BatchChatCompletionRequest, raw_request: Request): + raw_body = await raw_request.json() + _validate_chat_completion_raw_body(raw_body) handler = OmniBatchChat(raw_request) if handler is None: base_server = getattr(raw_request.app.state, "serving_tokenization", None)