fix(api): Reject query operands the column cannot take (backport #925) - #932
Merged
Merged
Conversation
A filter operand that does not match its column type reached Postgres
verbatim and the driver error surfaced to the client as a 500:
GET /projects?q=creation_time=[a~b]
-> 500, core: invalid input syntax for type timestamp: "a"
Same for /audit-logs?q=op_time=[x~y] and artifacts ?q=push_time=[2020~abc],
and for exact matches and or-lists against integer columns.
Record each model field type while parsing the model metadata and check
the operand against it before the filter is applied, returning a bad
request instead. The check is deliberately permissive so nothing that
works today stops working: text columns take any literal, number columns
take numerics within the column's own width, timestamp columns take the
documented format and the other forms Postgres reads, booleans take the
full Postgres token set, and fuzzy matches and __icontains are exempt
because both render as ILIKE, which casts the column to text.
Clean backport of the merged a39a4d7, which is #874 plus the review
fixups that landed on it. Replaces #909, which carried the superseded
#874 commit.
Signed-off-by: Prasanth Baskar <prasanth@8gears.com>
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
|
Preview images for this PR are available in
Verify a preview image: Verify SBOM attestation: |
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
1 participant
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.
Backport of #925 to
release-2.15. Replaces #909, which carried the superseded #874 commit.A search filter whose value did not fit the column returned 500, leaking the raw Postgres error. It should be 400.
What changed
Filter values are checked against the column's type before the query is built, and a mismatch returns 400 instead of reaching the database. This covers every endpoint that filters through
orm.QuerySetter.The check is deliberately permissive, so queries that work today keep working:
__icontainsare exempt, since they search as text2020-04-09 02:36:00format plus the other forms Postgres acceptsyes,onandnRelationship to #909
#909 backported #874, which was closed and continued as #925 after picking up review fixups — column-width checks, the boolean token set, the
__icontainsexemption, and a corrected DB test case. Rather than retrofit #909 in place, this is a fresh branch cut from the merged commit so the content is exactly what landed onmain.How it was produced
Cherry-pick equivalent of a39a4d7, the squashed merge of #925. All five touched files were byte-identical between
release-2.15and a39a4d7's parent before the pick, so nothing release-specific was overwritten.How it was verified
Built against the
release-2.15base, withlib/ormandlib/qgreen and the db-taggedpkg/project/daofilter-value suite passing against a freshly migrated Postgres on this branch: