search

ChurchCRM / CRM

ChurchCRM is an OpenSource Church CRM & Management Software.
https://ChurchCRM.io
MIT License
608 stars 429 forks source link

replace custom filter functions with php native extension #2244

Open crossan007 opened 7 years ago

DawoudIO commented 7 years ago

Can you give an example

crossan007 commented 7 years ago

we're using a custom, legacy FilterInput implementation: https://github.com/ChurchCRM/CRM/blob/master/src/Include/Functions.php#L314

PHP 5.2 and lower did not include the Filter extension by default, but all current versions should include it by default.

Validation: http://php.net/manual/en/filter.examples.validation.php Sanitization: http://php.net/manual/en/filter.examples.sanitization.php

crossan007 commented 7 years ago

explore Propel / Symphony validators: http://symfony.com/doc/current/validation.html and http://propelorm.org/documentation/behaviors/validate.html

crossan007 commented 7 years ago

@benbankes - may I solicit your professional expertise here - specifically as relating to #2296

benbankes commented 7 years ago

@crossan007 I don't have any experience with validation in Propel. My introduction to input validation was with other tools that haven't caused me enough pain to switch yet. As I would bet you are aware, just because Propel is being used, this doesn't mean that prepared statements are being used everywhere. For example https://github.com/ChurchCRM/CRM/blob/master/src/CSVCreateFile.php#L169 It looks like it is going to take a long time to ensure that all queries are using prepared statements before removing mysqli_real_escape_string from FilterInput(), but it also looks like it's necessary to do so from #2296

crossan007 commented 7 years ago

I'm thinking we could add a parameter to the FilterInput function call "SkipMysqliEscape." Default to false for full backward compatibility, but in places where we're filtering input that's being passed into Propel, we would just have to remember to ask FilterInput to skip the mysql escape.

crossan007 commented 7 years ago

And yes, I'm aware that there are many places where legacy queries are being executed.

@DawoudIO and I have been working on moving all legacy / direct queries to the Propel model.

It's a work in progress :-)

crossan007 commented 7 years ago

2622

github-actions[bot] commented 1 month ago

This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 5 days.

github-actions[bot] commented 1 week ago

This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 5 days.