libmbfl API changes in PHP 7.3

php.internals

Jan Ehrhardt

8 years ago
While trying to make the mailparse extension fit for PHP 7.3 I ran into changes in the libmbfl API. I cannot find any documentation on this change. The change was introduced in this commit by nikic: https://github.com/php/php-src/commit/b3c1d9d1118438a3dae3544447db2b39ca5cfa25 To be specific: https://github.com/php/php-src/commit/b3c1d9d1118438a3dae3544447db2b39ca5cfa25#diff-929fd74a8732776fa8776939a9b907d4L71 It broke the code of the mailparse extension at this line: https://github.com/php/pecl-mail-mailparse/blob/master/php_mailparse_mime.c#L910 Shouldn't the be documented? And/or is there an easy fix?
-- Jan

Jan Ehrhardt

8 years ago
Jan Ehrhardt in php.internals (Tue, 10 Jul 2018 10:30:33 +0200):
>The change was introduced in this commit by nikic: >https://github.com/php/php-src/commit/b3c1d9d1118438a3dae3544447db2b39ca5cfa25
What strikes me: this change was made to the master branch almost a year ago, at the time PHP 7.2.0 Beta 1 was released. Weird moment for a BC break.
-- Jan

Christoph Becker

8 years ago
On 10.07.2018 at 10:55, Jan Ehrhardt wrote:
> Jan Ehrhardt in php.internals (Tue, 10 Jul 2018 10:30:33 +0200): >> The change was introduced in this commit by nikic: >> https://github.com/php/php-src/commit/b3c1d9d1118438a3dae3544447db2b39ca5cfa25 > > What strikes me: this change was made to the master branch almost a year > ago, at the time PHP 7.2.0 Beta 1 was released. Weird moment for a BC > break.
It seems to me that is actually the best time regarding our release cycle: pushing breaking changes to *master* as early as possible after a release branch has been forked, so there would still be time to revert or to cater to related issues.
-- Christoph M. Becker

Christoph Becker

8 years ago
On 10.07.2018 at 10:30, Jan Ehrhardt wrote:
> While trying to make the mailparse extension fit for PHP 7.3 I ran into > changes in the libmbfl API. I cannot find any documentation on this > change. > > The change was introduced in this commit by nikic: > https://github.com/php/php-src/commit/b3c1d9d1118438a3dae3544447db2b39ca5cfa25 > > To be specific: > https://github.com/php/php-src/commit/b3c1d9d1118438a3dae3544447db2b39ca5cfa25#diff-929fd74a8732776fa8776939a9b907d4L71 > > It broke the code of the mailparse extension at this line: > https://github.com/php/pecl-mail-mailparse/blob/master/php_mailparse_mime.c#L910 > > Shouldn't the be documented? And/or is there an easy fix?
Have you tried to convert the first two arguments via mbfl_no2encoding()[1]? Anyhow, it seems to me that we should document why “--with-libmbfl is no longer available” (UPGRADING); because the bundled libmfl is now mandatory, since it has been changed, and at least to superficially add some respective notes to UPGRADING.INTERNALS. [1] <https://github.com/php/php-src/blob/php-7.3.0alpha3/ext/mbstring/libmbfl/mbfl/mbfl_encoding.c#L271-L285>
-- Christoph M. Becker

Jan Ehrhardt

8 years ago
"Christoph M. Becker" in php.internals (Tue, 10 Jul 2018 12:04:10 +0200):
>Have you tried to convert the first two arguments via mbfl_no2encoding()[1]?
It compiles, but segfaults in 1 test on this line https://github.com/php/pecl-mail-mailparse/blob/master/mailparse.c#L1466 Unhandled exception at 0x000007FEEC6E83C2 (php7.dll) in php.exe: 0xC0000005: Access violation reading location 0x000000060BE02800. occurred Relevant part of the backtrace: [Inline Frame] php7.dll!zend_mm_alloc_small(_zend_mm_heap *) Line 1285 [Inline Frame] php7.dll!zend_mm_alloc_heap(_zend_mm_heap * heap, unsigned __int64) Line 1356 php7.dll!_emalloc(unsigned __int64 size) Line 2496 [Inline Frame] php_mailparse.dll!zend_string_alloc(unsigned __int64) Line 133 [Inline Frame] php_mailparse.dll!zend_string_init(const char *) Line 155 php_mailparse.dll!mailparse_get_part_data(_php_mimepart * part, _zval_struct * return_value) It passes all tests in PHP 7.2.8 RC1. I will make a PR and continue the discussion there.
-- Jan

Jan Ehrhardt

8 years ago
Jan Ehrhardt in php.internals (Tue, 10 Jul 2018 15:39:39 +0200):
>"Christoph M. Becker" in php.internals (Tue, 10 Jul 2018 12:04:10 >+0200): >>Have you tried to convert the first two arguments via mbfl_no2encoding()[1]? > >It compiles, but segfaults in 1 test on this line >https://github.com/php/pecl-mail-mailparse/blob/master/mailparse.c#L1466
For anyone interested: it segfaults on Windows and Linux. No explication yet: https://github.com/php/pecl-mail-mailparse/pull/5#issuecomment-403990392 and following comments by Remi.
-- Jan

Nikita Popov

8 years ago
On Tue, Jul 10, 2018 at 10:30 AM, Jan Ehrhardt <phpdev@ehrhardt.nl> wrote:
> While trying to make the mailparse extension fit for PHP 7.3 I ran into > changes in the libmbfl API. I cannot find any documentation on this > change. > > The change was introduced in this commit by nikic: > https://github.com/php/php-src/commit/b3c1d9d1118438a3dae3544447db2b > 39ca5cfa25 > > To be specific: > https://github.com/php/php-src/commit/b3c1d9d1118438a3dae3544447db2b > 39ca5cfa25#diff-929fd74a8732776fa8776939a9b907d4L71 > > It broke the code of the mailparse extension at this line: > https://github.com/php/pecl-mail-mailparse/blob/master/ > php_mailparse_mime.c#L910 > > Shouldn't the be documented? And/or is there an easy fix? >
Thanks for pointing this out, this should be added to UPGRADING.INTERNALS. mbstring and libmbfl underwent some larger refactorings in PHP 7.3, to make its performance less abysmally bad. The change you are running into is that many APIs have changed from working with enum mbfl_no_encoding to const mbfl_encoding *. You will have to mirror these changes: For example, instead of passing mbfl_no_encoding_8bit to a function, you use &mbfl_encoding_8bit. Instead of calling mbfl_name2no_encoding(part->content_transfer_encoding) you instead use mbfl_name2encoding(part->content_transfer_encoding) and so on. Nikita