Fix: validate $filter before streaming response (return 400 not malformed 200)#873
Open
davetha wants to merge 1 commit into
Open
Fix: validate $filter before streaming response (return 400 not malformed 200)#873davetha wants to merge 1 commit into
davetha wants to merge 1 commit into
Conversation
When $filter references an unknown property or is syntactically invalid,
Lodata previously started writing the HTTP 200 response body
({"@odata.context":"...","value":[) before the filter was parsed, then
injected an OData-error trailer mid-stream. Clients received a malformed
response that was neither a valid OData error (HTTP 400) nor valid JSON.
Per the OData spec, an invalid $filter must return HTTP 400 Bad Request
before any response body is sent.
Add assertValidFilter() to EntitySet, mirroring the existing
assertValidOrderBy() pattern. It eagerly calls generateTree() on the
filter expression — using the entity set's own filter parser so custom
parsers are respected — and throws ParserException (a BadRequestException)
if the expression is invalid. The call site in get() is before
setResourceCallback(), so the exception propagates before the StreamedResponse
callback is registered or invoked.
Valid requests are unaffected: the filter tree is parsed once here for
validation, then again inside generateWhere() when the query executes.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
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
When
$filterreferences an unknown property (e.g.$filter=BadField eq 'value'), Lodata starts streaming the HTTP 200 response before evaluating the filter. The filter parse error then occurs mid-stream, injecting an error JSON fragment into the already-started response body. The client receives malformed output that is neither a valid OData error response (HTTP 400) nor valid JSON.Per the OData specification, an invalid
$filtershould return HTTP 400 Bad Request before any response body is sent.Changes
Added
assertValidFilter()inEntitySet::get(), called immediately after the existingassertValidOrderBy()and before theStreamedResponsecallback is registered. This eagerly parses the filter expression using the entity set's own filter parser, causing anyParserException(which extendsBadRequestException) to be thrown before streaming begins.How it works
The call chain for a GET request is:
get()— validates orderby, now validates filter, then registers the streaming callbackemitJson()→query()→generateWhere()→generateTree()With this fix, an invalid
$filterthrows at step 1, beforeStreamedResponse::sendContent()begins writing. The exception propagates through Laravel's normal error handling and produces a clean HTTP 400.Impact
$filternow returns proper HTTP 400 instead of malformed HTTP 200assertValidOrderBy()pattern exactlygenerateWhere()) — negligible cost for correctnessParserExceptionalready maps to HTTP 400Testing
assertValidOrderBy()precedent