Conversation

yajra

Fix #2901

@yajrayajra marked this pull request as draft November 17, 2022 07:22
@yajrayajra marked this pull request as ready for review November 18, 2022 07:21
@yajra

Note: need to review as encoding the keyword is a change in behavior and might cause a breaking change.

@sonarqubecloud

SonarCloud Quality Gate failed.    Quality Gate failed

BugA0 Bugs
VulnerabilityA0 Vulnerabilities
Security HotspotA0 Security Hotspots
Code SmellA0 Code Smells

No Coverage informationNo Coverage information
48.1%48.1% Duplication

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.

@yajra

@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'])

@dyaskur

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

@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?

@yajra

@aravael, would you be able to test this and see the impact on your existing project? Would you approve this PR?

@aravael

Hi @yajra . Recently managed to test it, can't approve this . 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.

Sign up for free to join this conversation on . Already have an account? Sign in to comment
None yet

Successfully merging this pull request may close these issues.