Skip to content

Conversation

@betsyecastro
Copy link
Contributor

@betsyecastro betsyecastro commented Dec 9, 2024

Implements ValidSearchString validation rule on ProfileSearchRequest to fix Sentry issue 6S - Array to string conversion and to improve the input validation on the search field.

Github issue #48

@betsyecastro betsyecastro added the 🐛 bug Something isn't working label Dec 9, 2024
@betsyecastro betsyecastro requested a review from wunc December 9, 2024 21:19
@betsyecastro betsyecastro self-assigned this Dec 9, 2024
@shukla-m shukla-m self-requested a review January 9, 2025 22:27
Copy link

@shukla-m shukla-m left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@betsyecastro I reviewed the code and tested it locally and the string validation works as expected. Additional validations can be added as needed, but this update works for validating that the search input value is a string.

cc: @wunc

@betsyecastro
Copy link
Contributor Author

@wunc I used the htmlspecialchars() function with the FILTER_FLAG_NO_ENCODE_QUOTES flag to convert any unwanted special characters that passed the regex validation into their HTML-escaped equivalents. However, I’ve been discussing with @shukla whether this is the most appropriate approach for preventing SQL injection, since we need to allow apostrophes for searchable input — which means they’ll still be included in the $search wildcard used in the query.

What are your thoughts on this? If you’re available Friday or Monday, perhaps we could review it together with Shukla. Thank you.

wunc added 2 commits November 12, 2025 10:03
- adds max length of 100
- regex: use unicode character properties
- formatting
@wunc wunc merged commit c652333 into develop Nov 12, 2025
1 of 2 checks passed
@wunc wunc deleted the profile-search-request branch November 12, 2025 16:55
@wunc wunc mentioned this pull request Nov 12, 2025
13 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🐛 bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants