-
Notifications
You must be signed in to change notification settings - Fork 399
🐛(backend) validate user search input data #1353
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds validation for user search input data to ensure queries are between 5 and 254 characters. The validation prevents issues with the Levenshtein distance algorithm which doesn't accept more than 254 characters, and aligns with the email field's max length of 254.
- Implemented a
UserSearchFilter
to validate query parameter length constraints - Updated user search endpoint to return 400 status with validation errors for invalid queries
- Added comprehensive test coverage for both short and long query validation
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
File | Description |
---|---|
src/backend/core/api/filters.py | Added UserSearchFilter class with min/max length validation for query parameter |
src/backend/core/api/viewsets.py | Integrated the new filter to validate search queries and return proper error responses |
src/backend/core/tests/test_api_users.py | Updated existing tests and added new test for long query validation |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
|
||
def test_api_users_list_query_long_queries(): | ||
""" | ||
Queries longer than 255 characters should return an empty result set. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The comment states queries longer than 255 characters should return an empty result set, but the implementation actually returns a 400 error with validation message. The comment should be updated to reflect the actual behavior: 'Queries longer than 254 characters should return a validation error.'
Queries longer than 255 characters should return an empty result set. | |
Queries longer than 254 characters should return a validation error. |
Copilot uses AI. Check for mistakes.
factories.UserFactory(email="[email protected]") | ||
factories.UserFactory(email="[email protected]") | ||
|
||
query = "a" * 244 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] The test creates a 244-character string and appends '@example.com' (12 characters) for a total of 256 characters, but this logic is not immediately clear. Consider using a more explicit approach like query = 'a' * (254 + 1 + 1)
with a comment explaining the calculation (254 max + 1 to exceed + 1 for '@') or calculate the exact length needed.
query = "a" * 244 | |
# The maximum allowed query length is 254 characters. | |
# '@example.com' is 12 characters, so we construct a string that exceeds the limit: | |
# (254 max) + 1 to exceed + 1 for '@' = 256 total length. | |
query = "a" * (254 + 1 + 1 - len("@example.com")) |
Copilot uses AI. Check for mistakes.
Size Change: +4 B (0%) Total Size: 3.65 MB
|
6563e1d
to
356b84b
Compare
Only the input data min length was checked. We also have to check the mex length because the levenshtein dos not accept more than 254 characters and the email field has a max length of 254
When we create a new user in the demo environment, the email address will now follow the format [email protected] instead of [email protected]. "user" was only 4 characters long, it created failing tests in the e2e suite.
254 characters should be sufficient for most of our usecases. Limit input search to 254 characters to prevent errors caused by overly long email addresses.
356b84b
to
4f2e07f
Compare
Purpose
Only the input data min length was checked. We also have to check the max length because the levenshtein does not accept more than 254 characters, and the email field has a max length of 254.
Proposal
Fixes #1348