Skip to content

QA - Bug fix - filter_input_array - solution 2#13804

Merged
ndossche merged 2 commits intophp:masterfrom
juan-morales:qa-bugfix-filter_input_array-2
Apr 6, 2024
Merged

QA - Bug fix - filter_input_array - solution 2#13804
ndossche merged 2 commits intophp:masterfrom
juan-morales:qa-bugfix-filter_input_array-2

Conversation

@juan-morales
Copy link
Contributor

@juan-morales juan-morales commented Mar 25, 2024

relates to #13805

@kocsismate (because you are assigned as reviewer in the other pull request)

coverage:

after-2

@juan-morales juan-morales requested a review from kocsismate March 26, 2024 13:21
@kocsismate kocsismate requested review from Girgias and ndossche March 26, 2024 22:21
Copy link
Member

@ndossche ndossche left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What a mess.

It seems like the code was kinda designed to be able to do FILTER_DEFAULT | FILTER_NULL_ON_FAILURE because their bits are exclusive, but indeed it doesn't work like that in practice and gives a warning due to the PHP_FILTER_ID_EXISTS.

The whole "flags" thing looks copy-pasted from elsewhere from the file, but indeed the meaning of this array is different.

And indeed, the code will always return NULL now.
The code change right now is correct as it will have the same behaviour as previously, just simpler.
I think this approach is okay.

@juan-morales
Copy link
Contributor Author

@Girgias @kocsismate what do you think ?

@juan-morales
Copy link
Contributor Author

@nielsdos is it normal to take so long for something like this ? (sorry for being anxious)

@kocsismate
Copy link
Member

Sorry, I am very busy with another task which of course takes longer than expected.. but I will have another look as soon as possible. Anyway, I completely trust Niels' opinion, so I am ok with merging this as-is even though I'd have preferred to try to find a way to really fix the underlying issue).

@ndossche
Copy link
Member

ndossche commented Apr 6, 2024

I'll merge this as-is since this is a correct change. Would be great to find a nice way to solve the fundamental problem here, but the change as-is is correct.

@ndossche ndossche merged commit c96b975 into php:master Apr 6, 2024
@juan-morales
Copy link
Contributor Author

Thanks a lot people

@juan-morales juan-morales deleted the qa-bugfix-filter_input_array-2 branch December 1, 2025 13:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants