Modernization of CDR - #53
Conversation
|
This appears to be AI-generated and contains multiple potential SQL injection errors. |
|
@chrsmj Can you give me an example just to check? It's just to see if claude is able to fixe it. |
|
No I am not going to debug your AI generated code. |
|
Well. As you like. |
|
Are there any unit tests being added ? |
|
Hey @danardf , just a quick pointer on the SQL injection topic. Check out how $_REQUEST['sort'] is used to build the ORDER BY clause in the new getCdrData function. Directly using request parameters in that part of a query is a common risk. A whitelist for allowed column names is usually the safest approach there. Hope that's a helpful starting point! |
|
@mrpbueno hi |
Interestingly, the current Cdr.class.php has the same SQL injection issues, AI didn't seem to create those Franck just used what already existed. So kudos you just discovered existing SQL injection issues in the CDR module. |
|
I think it's fixed now |
|
Hi |
|
@kguptasangoma Hi |
|
Hi @danardf did not get chance to look into this one. Needs to kick off the QA also. Thanks |
|
@kguptasangoma Ok update me asap. please. |
|
May be we can ask community help also to test the PR. |
|
As you want. |
|
Hi @danardf,
From the failed request: From the successful request: One more usability note: |
|
@hannes427 Thank you so much fro your feedback. I will try to fixe these issues asap. just need to be free a while. ;) If there is only this kind of issues, That fine.... |
|
@danardf You're welcome! I'm not sure if this is already obvious, but the issue where startdate and from_day/from_month/from_year (and respectively enddate and to_day/to_month/to_year) contain different values also occurs the other way around. If you change the date only under “Advanced Search Options” and not in the “Quick Date Range Picker”, different date values are sent in the request, and not all entries are found. For example, if you set the start date under the Advanced search options to 2021-01-01 and the end date to 2025-10-18 without changing anything under the “Quick Date Range Picker”, the following values are sent via the HTTP request (truncated): You obviously have to enter the desired dates in both fields — that is, under the “Advanced Search Options” and in the “Quick Date Range Picker.” |
|
@danardf Just wondering what happened to the state of this module? IMHO a huge improvement compared to what we have now ... |
|
@jacotec Hi. Yes indeed, I stopped to worked on it for a while. I was busy on somthing else. |
|
No problem, just want to ask because I like the approach. Thanks for the update! |
Rewrites the admin CDR report page around bootstrap-table with server-side search, sort and pagination instead of the legacy form-based page, and brings the branch back in sync with release/16.0 (16.0.46.27 -> 16.0.50). - New AJAX grid endpoint (getCdrData) replacing page.cdr.php's inline search/report logic - Whitelist-based sort-column validation on the new endpoint to prevent SQL injection via the "sort" request parameter - Quick Date Range Picker: independent per-calendar navigation, month/year dropdowns, correct start/end ordering regardless of click order, and consistent sync with the Advanced Search Options date fields (previously could silently narrow results with a stale range) - Scribe UCP transcription download link - GraphQL fetchAllCdrs pagination/date-filter fixes - Expanded security regression test coverage (utests/) See PR FreePBX#53 for the full review history. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
0fe9aa4 to
ecb79ec
Compare
|
Hi Everyone. @kapilgupta01 @jacotec @hannes427 @blazestudios97 If you can test asap please. Have a nice summer |
I modernized this old module which was ugly for a long time ago. Now it looks better.