[NFR]: Make mbstring required

[alexbusu]

Describe the bug
A php -r 'echo \Phalcon\Version::get();' returns without errors.
Trying to run the application, fails instead. Core is dumped by segmentation fault.

To Reproduce
I'm not sure what component causes the core dump, but I'll try to figure out.
Here is the coredump

(gdb) bt
#0 0x00005555557dd1a5 in zval_get_string_func ()
#1 0x00005555557ec1e8 in zend_get_callable_name_ex ()
#2 0x00005555557d735c in zend_call_function ()
#3 0x00007fffe71d4a93 in ?? () from /usr/lib/php/20190902/phalcon.so
#4 0x00007fffe71eba4f in ?? () from /usr/lib/php/20190902/phalcon.so
#5 0x00007fffe72012fa in ?? () from /usr/lib/php/20190902/phalcon.so
#6 0x00005555557d6f49 in zend_call_function ()
#7 0x00007fffe71d4a93 in ?? () from /usr/lib/php/20190902/phalcon.so
#8 0x00007fffe71e579a in ?? () from /usr/lib/php/20190902/phalcon.so
#9 0x00007fffe731039e in ?? () from /usr/lib/php/20190902/phalcon.so
#10 0x00005555557d6f49 in zend_call_function ()
#11 0x00007fffe71d4a93 in ?? () from /usr/lib/php/20190902/phalcon.so
#12 0x00007fffe71e579a in ?? () from /usr/lib/php/20190902/phalcon.so
#13 0x00007fffe7388ef8 in ?? () from /usr/lib/php/20190902/phalcon.so
#14 0x00005555557d6f49 in zend_call_function ()
#15 0x00007fffe71d4a93 in ?? () from /usr/lib/php/20190902/phalcon.so
#16 0x00007fffe71e579a in ?? () from /usr/lib/php/20190902/phalcon.so
#17 0x00007fffe73debb0 in ?? () from /usr/lib/php/20190902/phalcon.so
#18 0x00005555558721c7 in execute_ex ()
#19 0x00005555557d709c in zend_call_function ()
#20 0x00007fffe71d3a5f in ?? () from /usr/lib/php/20190902/phalcon.so
#21 0x00007fffe75ab2bd in ?? () from /usr/lib/php/20190902/phalcon.so
#22 0x00005555557d6f49 in zend_call_function ()
#23 0x00007fffe71d4a93 in ?? () from /usr/lib/php/20190902/phalcon.so
#24 0x00007fffe71e579a in ?? () from /usr/lib/php/20190902/phalcon.so
#25 0x00007fffe74849dd in ?? () from /usr/lib/php/20190902/phalcon.so
#26 0x00005555557d6f49 in zend_call_function ()
#27 0x00007fffe71d4a93 in ?? () from /usr/lib/php/20190902/phalcon.so
#28 0x00007fffe71e579a in ?? () from /usr/lib/php/20190902/phalcon.so
#29 0x00007fffe73f6dbe in ?? () from /usr/lib/php/20190902/phalcon.so
#30 0x0000555555872549 in execute_ex ()
#31 0x0000555555873be3 in zend_execute ()
#32 0x00005555557e6033 in zend_execute_scripts ()
#33 0x0000555555783900 in php_execute_script ()
#34 0x0000555555875b26 in ?? ()
#35 0x00005555556400fc in ?? ()
#36 0x00007ffff5cdf830 in __libc_start_main (main=0x55555563fc80, argc=3, argv=0x7fffffffe328, init=, fini=, rtld_fini=, stack_end=0x7fffffffe318)
at ../csu/libc-start.c:291
#37 0x0000555555640219 in _start ()

Steps to reproduce the behavior:
Create config.ini file with the following content:

[some]
mode = 2 ;1-PDO, 2-native extension, 0-disabled

Then a test file t.php

<?php
$config = new \Phalcon\Config\Adapter\Ini('config.ini');
var_dump($config);

By running php t.php the segfault happens. This is because of that comment after the value.

Details

• Phalcon version: 4.0.2
• PHP Version: 7.4.1
• Operating System: Ubuntu 16.04.5 LTS
• Installation type: installing via package manager (from ppa:ondrej/php)
• Zephir version (if any): -
• Server: Nginx | CLI

Additional context
I'm trying to migrate from Phalcon 3 to v4 and the service container defines some services based on php version (const IS_PHALCON_4 = PHP_VERSION_ID >= 70400;). Although the unused imports (like Cache Backends of Phalcon 3) are not checked/autoloaded, it might be an issue in case of Phalcon4?
Brb with more info as I get it.

[ruudboon]

I'm wondering if the ppa:ondrej/php repo is still holding the 3.4 release. In that case we should change the docs. Can you try installing using package cloud?

https://docs.phalcon.io/4.0/en/installation#deb-based-distributions-debian-ubuntu-etc

[ruudboon]

[alexbusu]

I have some updates.
Steps to reproduce the behavior:
Create config.ini file with the following content:

[some]
mode = 2 ;1-PDO, 2-native extension, 0-disabled

Then a test file t.php

<?php
$config = new \Phalcon\Config\Adapter\Ini('config.ini');
var_dump($config);

By running php t.php the segfault happens. This is because of that comment after the value.

This is not the only case. I chase for more.

[alexbusu]

Btw,

apt policy php7.4-phalcon
php7.4-phalcon:
  Installed: 4.0.2-884+php7.4
  Candidate: 4.0.2-884+php7.4
  Version table:
 *** 4.0.2-884+php7.4 500
        500 https://packagecloud.io/phalcon/stable/ubuntu xenial/main amd64 Packages
        100 /var/lib/dpkg/status

[ruudboon]

@alexbusu can you drop the output of php -m here?

[alexbusu]

@alexbusu can you drop the output of php -m here?

$ php -m
[PHP Modules]
apcu
calendar
Core
ctype
curl
date
dom
exif
FFI
fileinfo
filter
ftp
gettext
hash
iconv
igbinary
json
libxml
mbstring
mcrypt
memcached
msgpack
mysqli
mysqlnd
openssl
pcntl
pcre
PDO
pdo_mysql
phalcon
Phar
posix
psr
readline
redis
Reflection
session
shmop
SimpleXML
sockets
sodium
SPL
standard
sysvmsg
sysvsem
sysvshm
tokenizer
xml
xmlreader
xmlwriter
xsl
Zend OPcache
zip
zlib

[Zend Modules]
Zend OPcache

But some things have changed, and the good news is that I have no segfault, even with that example that I reported. What I noticed is that when the segfault did not happen I got the error of calling the missing mb_strtolower function. By adding the mbstring the segfaults did not happen anymore, everything runs well now.

If this extension is required, can you require it explicitly as you do for the psr extension? It would help avoiding such messy reports.

[ruudboon]

It will depend on what part of the framework you will be using if mbstring is required. Maybe mbstring is so general we should add it as dependency, on the other hand we don't want to force extensions people don't need.
We do have a list here: https://docs.phalcon.io/4.0/en/installation#software

@sergeyklay Any thought on adding mbstring as dependency?

[alexbusu]

Of course, that segfault should be treated someway. Either throw an Exception when mbstring is not installed/enabled, or just emit warnings and continue with strtolower (or other non-mb_ functions).

[sergeyklay]

It will depend on what part of the framework you will be using if mbstring is required. Maybe mbstring is so general we should add it as dependency, on the other hand we don't want to force extensions people don't need.
We do have a list here: https://docs.phalcon.io/4.0/en/installation#software

@sergeyklay Any thought on adding mbstring as dependency?

I'm OK with adding mbstring as dependency.

/cc @niden

[niden]

If I understand this correctly it is caused by the mbstring extension missing. In various spots in the code we check if the mbstring extension is available and use its methods - otherwise use alternatives. I am guessing that we d

Adding mbstring would definitely enhance the framework. All for it.

It will also help us by removing different unnecessary IF statements to check if mbstring exists.

[ruudboon]

Can we still do this in 4.0 branch or is adding a dependency something for 4.1? There could be situations where people are running without mbstring. SemVer guru's?

[sergeyklay]

Let's do it in 4.1