• 0 Vote(s) - 0 Average
  • 1
  • 2
  • 3
  • 4
  • 5
Should $request->getPost() and others modify data even when no filtering specified?

#1
I am developing an application which needs to handle a great number of forms.

These forms are large, and many of the fields are not mandatory.

One of the steps of processing these forms is converting all empty strings to null, in order to avoid having multiple "no-value" values stored in the db (such as '' and null).

The conversion is quite simple, but having to do this every time in the Controller (or elsewhere) is sort of annoying and error prone, so I thought I'd write up a CI4 Filter for the job.

Long story short, after I replace the empty strings with null and modify the $request via setGlobal('post', $modified_data), when I retrieve the data back inside the controller via $request->getPost() all nulls are converted back to string.

I believe the culprit is filter_var($value, $filter, $flags), which will convert nulls to empty string even if FILTER_UNSAFE_RAW (which according to php docs should let any value through).

There is an easy enough fix which is to test values for null before passing them through filter_var, and this seems to be restricted to the fetchGlobal method inside ResponseTrait.

Would this be an acceptable patch? I'm happy to provide a PR.

Eager to hear your thoughts,

André
Reply

#2
It sounds like the source of the issue is still unclear. I would certainly be glad to see a Bug Report and corresponding Pull Request with a fix, but I think you might need to demonstrate the issue first. My recommendation would be to peruse the framework tests and find a place to add a test that demonstrates the problem, then you can go about fixing it.
Reply

#3
(04-08-2021, 07:23 AM)MGatner Wrote: It sounds like the source of the issue is still unclear. I would certainly be glad to see a Bug Report and corresponding Pull Request with a fix, but I think you might need to demonstrate the issue first. My recommendation would be to peruse the framework tests and find a place to add a test that demonstrates the problem, then you can go about fixing it.

How is it not clear? Did you read what I wrote? The reason this is in CI4 Discussion is that I don't know if the proposed solution (also clearly stated in my message) violates any foundational goals or deviates from intended milestones.

$request->getPost(...) will convert nulls to empty strings, which seems to me to be undesired and unintended.

How cleared can it get?
Reply

#4
(04-01-2021, 02:39 PM)andre.tannus Wrote: I believe the culprit is filter_var($value, $filter, $flags), which will convert nulls to empty string even if FILTER_UNSAFE_RAW (which according to php docs should let any value through).

I did read your post, and your above uncertainty is what led me to ask my questions. I have not looked at the code, but if we are already using a flag that is supposed to allow null values then it seems to me the source of the issue is still uncertain. If you feel confident in your assessment and solution you are very welcome to submit a PR. I cannot say without seeing the changes whether this would be considered a “breaking change” but it sounds to me like you’ve discovered a bug so a fix is in order.
Reply

#5
(04-09-2021, 04:26 AM)MGatner Wrote:
(04-01-2021, 02:39 PM)andre.tannus Wrote: I believe the culprit is filter_var($value, $filter, $flags), which will convert nulls to empty string even if FILTER_UNSAFE_RAW (which according to php docs should let any value through).

I did read your post, and your above uncertainty is what led me to ask my questions. I have not looked at the code, but if we are already using a flag that is supposed to allow null values then it seems to me the source of the issue is still uncertain. If you feel confident in your assessment and solution you are very welcome to submit a PR. I cannot say without seeing the changes whether this would be considered a “breaking change” but it sounds to me like you’ve discovered a bug so a fix is in order.


Well, from my tests, filter_var with FILTER_UNSAFE_RAW does convert nulls to empty strings. This may have stayed under the radar since it is being used to filter POST data, which is always in string format. One of my concerns is people implicitly relying on the data being composed exclusively of strings but that will remain true as long as they are not fiddling with it in a Filter, but again, if they are, and rely on this cast to prevent, say, nulls from being sent to the db, then it will break things somewhere.

Shall I roll up my sleeves?
Reply

#6
I see. I think the problem is that we cannot just suddenly start passing back NULL as this could mess up other developers. You may be able to fix this in your application by passing FILTER_FLAG_EMPTY_STRING_NULL as an additional $flag:
https://3v4l.org/puPvf

Note though that this also causes '' to return NULL, which may or may not be desirable for your use case.

It is odd to me that filter_var with “unsafe raw” converts null values to strings but that’s on PHP, not CodeIgniter.
Reply


Digg   Delicious   Reddit   Facebook   Twitter   StumbleUpon  


  Theme © 2014 iAndrew  
Powered By MyBB, © 2002-2021 MyBB Group.