-
-
Couldn't load subscription status.
- Fork 855
fix: encode search keyword #2903
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
Note: need to review as encoding the keyword is a change in behavior and might cause a breaking change.
Co-authored-by: Mathieu FERRE <Arkhas@users.noreply.github.com>
This reverts commit 077d2e8.
9d13187 to
d2ab963
Compare
SonarCloud Quality Gate failed. Quality Gate failed
Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells
No Coverage information No Coverage information
48.1% 48.1% Duplication
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.
This change will make we can not search non encoded special chars. an example we have data "Hand & Soulder"(no encode) and we search using "&", and the data will not shown.
My suggestion is using attribute like searchable in datatable options.
an example:
{data: 'name', name: 'name',searchable: true, is_encoded:true},
and if is_encoded:true, we use e() function, otherwise return without encode.
@dyaskur thanks for the review. However, adding is_encoded:true in the script will not affect the serverSide implementation as the js library will not send it back in the ajax request.
How about if we add another control on a column level that toggles encoded search? Maybe something like:
->encodedSearch(['col1', 'col2'])
I see, I thought datatable will send all column attribute to the ajax request. If can't send by client side, we need to set it on server side. And I think your idea looks good.
aravael
commented
Feb 16, 2023
@yajra , recently came up with a similar solution while debugging a weird behavior with the collection as a source. This change adds equality to a search input and modified collection at DataProcessor escapeRow which uses laravel's e() func.
Any estimates on when this will be merged?
@aravael, would you be able to test this and see the impact on your existing project? Would you approve this PR?
aravael
commented
Feb 28, 2023
Hi @yajra . Recently managed to test it, can't approve this patch. Though it fixes the collection search, it breaks the builder source. Since we sanitize the input with e(), the database data are still not modified, therefore not found the result.
It works with a collection because both the collection and input sanitized. In case of builder source it doesn't work that way. Need to process that out.
Fix #2901