reactphp/dns

Improve compatibility with deprecated mbstring.func_overload

brstgt opened this issue ยท 5 comments

Could you please check for the existance of mbstring extension and use the mb_* versions of strlen/substr with 'ascii' as encoding parameter?

clue commented

Hi @brstgt! I'm not sure I follow what you're trying to do and what your issue is, as it's a bit unclear what you're trying to achieve from the description you've posted.

May I ask you provide a simple gist of what you're trying to achieve? Thanks!

This library is in fact compatible with ext-mbstring. It sounds like you may be using the deprecated mbstring.func_overload ini option, see http://php.net/manual/en/mbstring.overload.php for more details.

You are right, I am referring to mbstring function overloading. It is deprecated as of 7.2 which is not released yet, so it is maybe not that uncommon that people out there use the func overloading.
Unfortunately we are using it (for years). Of course, I will try to abandon it but until we can do that I have to mess with the source of react/dns to make it work as expected.

There are only a handful usages of string functions in the DNS resolver. It should be pretty easy to handle that correctly for both regular string functions and overloaded string functions.

clue commented

It is deprecated as of 7.2 which is not released yet, so it is maybe not that uncommon that people out there use the func overloading.
Unfortunately we are using it (for years). Of course, I will try to abandon it but until we can do that I have to mess with the source of react/dns to make it work as expected.

Is turning this off for you an option? I realize that there are a number of installations out there that may rely on this. There is also a reasonable number of OSS projects that decided against implementing a work-around. Also, the deprecation is not exactly unexpected: http://marc.info/?l=php-internals&m=133548185505478&w=2

There are only a handful usages of string functions in the DNS resolver. It should be pretty easy to handle that correctly for both regular string functions and overloaded string functions.

PRs would be much appreciated and we may look into this ๐Ÿ‘ I'm not to decide this on my own, but if the changeset is reasonable, I'm not opposed to getting this in.

Hi @clue,

You convinced me of refactoring our own code to not use mbstring overloadings.

Thanks anyway!