brefphp / bref

Serverless PHP on AWS Lambda
https://bref.sh
MIT License
3.15k stars 364 forks source link

Extension intl uses old ICU version #1534

Open korridor opened 1 year ago

korridor commented 1 year ago

Description:

I use the PHP intl extension to format a number with currency. This does not work as expected, which is due to a bug in the ICU library, which was fixed a few years ago. So, I looked at which version Bref uses.

According to the constant INTL_ICU_VERSION (docs) the version 50.2 is used. ICU version 50.2 was released in Apr 11, 2019. (more than 4 years old) -> Release note

How to reproduce:

Bref version: 1.7.23 Layers:

PHP version: 8.0.28

mnapoli commented 1 year ago

Bref 1.7.23 is not the latest version, do you see the same problem with Bref v2?

korridor commented 1 year ago

I don't have a Bref v2 version in production, but I just tested it with the docker image bref/php-81-fpm-dev:2 for the local setup and it uses the same version (50.2) there.


docker run -v ./php:/var/task/php -it bref/php-80-fpm-dev:2 php -a

The volume is for the php.ini to activate the intl extension.

var_dump(INTL_ICU_VERSION);
korridor commented 1 year ago

I just tested it with the PHP 8.1 image (bref/php-81-fpm-dev:2) as well and it also uses the same version.

mnapoli commented 1 year ago

Thank you, that's noted! We should upgrade the version in https://github.com/brefphp/aws-lambda-layers

PRs welcome!

korridor commented 1 year ago

Thanks! I just tried to update this package in the local Dockerfile with yum, but it says that it is already up-to-date. How do you normally update packages that are already bundled in the Amazon Linux 2 image?

mnapoli commented 1 year ago

Ah, this is probably Amazon Linux being outdated again :/

This is why we compile some dependencies, for example: https://github.com/brefphp/aws-lambda-layers/blob/main/php-80/Dockerfile#L72-L105 So that we get more up to date versions.

fredericgboutin-yapla commented 3 months ago

Yeah, it is so old that we cannot even use NumberFormatter::CURRENCY_ACCOUNTING to format money amounts correctly. So on top of security issues, there is this.

fredericgboutin-yapla commented 2 months ago

Ah, this is probably Amazon Linux being outdated again :/

This is why we compile some dependencies, for example: https://github.com/brefphp/aws-lambda-layers/blob/main/php-80/Dockerfile#L72-L105 So that we get more up to date versions.

Yeah, it definitely seems more like Amazon Linux is thoroughly outdated. I wonder how people can tolerate that? Because, yeah, updating with yum has no effect at all. In fact, there are other people suffering from the same issue and they don't like to understand the underlying problem - https://github.com/ovalhub/pyicu/issues/124 😅

Anyway, It seems that we should explicitly tell the major version we wish to use.

For example, there is a package for ICU v60 - https://pkgs.org/search/?q=libicu

yum install libicu60

> installs version 60.3-2.amzn2.0.4

The problem then is to install that version prior to compile PHP itself and linking it properly.

Yup, and it seems a bunch of other components like LIBXML2 probably need to be adjusted as well. Also, we must not forget libicu-devel that should be libicu60-devel-60.3-2.amzn2.0.1.x86_64 instead 😭

Wow...

mnapoli commented 2 months ago

TBH PHP 8.0 has been EOL for almost a year now, it's no longer updated.

I don't think we'll upgrade the ICU version in the layers. I'll mark this as closed here to clean up the tracker, feel free to continue the discussion (and we can always reopen if needed).

fredericgboutin-yapla commented 2 months ago

@mnapoli it applies to ALL php versions... this is a GENERIC issue with Amazon Linux image. The issue should be renamed to "Extension intl uses old ICU version"

mnapoli commented 2 months ago

Got it.