QA - mb_http_input - function returns FALSE for type 'L' or 'l'

[juan-morales]

Information

Extension: mbstring
Function: mb_http_input
Case: Parameter $type is 'L' or 'l' is provided and function returns false

Another reason to test this is that in the code there is a //TODO note on this case, suggesting that there might be a possibility that in the future instead of FALSE ... empty string might be returned ... is an open question developer left there.

I think is wort the effort to have a test on this, but is just my opinion.

Code coverage

Before

After

[cmb69]

Yeah, I think it makes sense to test this condition. @alexdowad, what do you think.

BTW, the underlying mbstring.http_input and friends have been deprecated long ago, but have still not been removed. Either we remove the settings with PHP 9, or we should undeprecate.

[juan-morales]

Yeah, I think it makes sense to test this condition. @alexdowad, what do you think.

BTW, the underlying mbstring.http_input and friends have been deprecated long ago, but have still not been removed. Either we remove the settings with PHP 9, or we should undeprecate.

I Am open to do What is needed on this change.

Thanks for the review and I stay tune on the conversation if any modification is needed

[alexdowad]

Yeah, I think it makes sense to test this condition. @alexdowad, what do you think.

BTW, the underlying mbstring.http_input and friends have been deprecated long ago, but have still not been removed. Either we remove the settings with PHP 9, or we should undeprecate.

I suggest that:

• The TODO note in the code should be deleted. It has doubtless been there for many years, without anything changing.
• A test should be added, confirming the existing behavior, as @juan-morales suggests.
• We should schedule when to delete the deprecated functionality.

[cmb69]

Thank you!

I have deleted the TODO comment now.

[alexdowad]

When should this functionality be removed? At the next major version bump?

[cmb69]

@alexdowad, yes (if we really want to remove it; see https://externals.io/message/103087).

[juan-morales]

thanks @cmb69 and @alexdowad for the support