Reject unknown fields from native API request bodies#790
Open
dongjangoon wants to merge 1 commit into
Open
Conversation
5bd192a to
93820d6
Compare
Add `#[serde(deny_unknown_fields)]` to the lorax-native HTTP request body structs so requests containing unknown or misplaced fields fail with a clear deserialization error instead of being silently ignored. This addresses the motivating case in predibase#478 (e.g. putting `api_token` at the top level instead of inside `parameters`). Structs covered: - GenerateRequest, CompatGenerateRequest, TokenizeRequest - EmbedRequest, CompatEmbedRequest - ClassifyRequest, BatchClassifyRequest, BatchEmbedRequest Deliberately NOT covered: - ChatCompletionRequest, CompletionRequest back the OpenAI-compatible /v1/chat/completions and /v1/completions endpoints, which conventionally tolerate unknown fields for forward-compat with the OpenAI spec. These structs already follow that convention (several `#[allow(dead_code)]` accept-but-ignore fields); adding deny_unknown_fields would reject standard OpenAI params lorax hasn't implemented yet (e.g. stream_options, parallel_tool_calls, max_completion_tokens) and break real clients. - JsonSchemaTool uses `#[serde(flatten)]`, which serde does not allow together with deny_unknown_fields. Closes predibase#478 Signed-off-by: dongjangoon <kdh0320j@gmail.com>
93820d6 to
9ef36c3
Compare
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.
What
Adds
#[serde(deny_unknown_fields)]to the lorax-native HTTP request body structs inrouter/src/lib.rs, so a request containing an unknown or misplaced field fails with a clear deserialization/validation error instead of being silently ignored.Closes #478.
Why
Per #478, unknown top-level fields are currently dropped silently. A common failure mode is putting
api_tokenat the top level instead of insideparameters, which then surfaces as a confusing "access denied" instead of a validation error. Rejecting unknown fields makes these mistakes obvious at the request boundary.Structs covered (8 native request bodies)
GenerateRequest,CompatGenerateRequest,TokenizeRequestEmbedRequest,CompatEmbedRequestClassifyRequest,BatchClassifyRequest,BatchEmbedRequestDeliberately NOT covered
ChatCompletionRequest/CompletionRequest— these back the OpenAI-compatible/v1/chat/completionsand/v1/completionsendpoints, which conventionally tolerate unknown fields for forward-compat with the OpenAI spec. These structs already follow that convention (several#[allow(dead_code)]accept-but-ignore fields), and locking them down would reject standard OpenAI params lorax hasn't implemented yet (e.g.stream_options,parallel_tool_calls,max_completion_tokens,top_logprobs) and break real OpenAI SDK clients. Happy to include them if you'd prefer strictness there too.JsonSchemaTool— uses#[serde(flatten)], which serde does not allow together withdeny_unknown_fields(it's the onlyflattenin the file, and it isn't a request body).Notes
GenerateParameters) could be a follow-up.cargo build/test; the change is a pure serde container-attribute addition with the oneflattenstruct excluded, so I'm relying on CI to confirm.