jstanden / cerb

Cerb is a fully customizable, web-based platform for enterprise communication and process automation. Create high volume shared inboxes. Integrate with any API-based service and automate nearly any repetitive digital workflow using the specialized KATA language and browser-based coding tools. Production: https://github.com/cerb/cerb-release/
https://cerb.ai/
Other
80 stars 38 forks source link

Conversation widget sometimes render empty (Ajax failure) #1746

Closed netdreamer closed 8 months ago

netdreamer commented 11 months ago

Hello, I found an issue on the rendering of the "Conversation" widget: sometimes it's completely blank.

By inspecting the web calls, I found out that this Ajax call for widget rendering fails with error 500: POST /admin/ajax.php?_log=profiles.invokeTab.39.renderWidget.97" This is the call trace in the web error log.

PHP Fatal error:  Uncaught TypeError: abs(): Argument #1 ($num) must be of type int|float, string given in /web/admin/vendor/ezyang/htmlpurifier/library/HTMLPurifier/UnitConverter.php:264
Stack trace:
#0 /web/admin/vendor/ezyang/htmlpurifier/library/HTMLPurifier/UnitConverter.php(264): abs()
#1 /web/admin/vendor/ezyang/htmlpurifier/library/HTMLPurifier/UnitConverter.php(178): HTMLPurifier_UnitConverter->round()
#2 /web/admin/vendor/ezyang/htmlpurifier/library/HTMLPurifier/Length.php(153): HTMLPurifier_UnitConverter->convert()
#3 /web/admin/vendor/ezyang/htmlpurifier/library/HTMLPurifier/AttrDef/CSS/Length.php(65): HTMLPurifier_Length->compareTo()
#4 /web/admin/vendor/ezyang/htmlpurifier/library/HTMLPurifier/AttrDef/CSS/Composite.php(39): HTMLPurifier_AttrDef_CSS_Length->validate()

So, the issue is on the HTMLPurifier library used by Cerb.

The failure is in the UnitConverter.php, function "round". There is no check that the passed value $n is really a number, before trying to to abs($n)

    private function round($n, $sigfigs)
    {
               $new_log = (int)floor(log(abs($n), 10)); // Number of digits left of decimal - 1
    ...

If, for some reason, round() is called with an invalid value, it crashes. I don't know if there is an updated HTMLPurifier library that fixes the issue by checking values.

At the moment, I temporary fixed it by patching the function with a check:

    private function round($n, $sigfigs)
    {
        if (!is_numeric($n)) { return $n; }
        $new_log = (int)floor(log(abs($n), 10)); // Number of digits left of decimal - 1

A different workaround, if no updated library is available, would be to to check the passed values before calling HTMLPurifier_AttrDef_CSS_Length->validate() in Cerb code...

netdreamer commented 11 months ago

Just for reference, I filed an issue also on the HTMLPurifier library (https://github.com/ezyang/htmlpurifier/issues/386), because I checked that the error is still in their GitHub code.

netdreamer commented 11 months ago

After some troubleshooting, I found that issue is related to locales, and is already fixed in master branch of the HTMLPurifier library (by commit https://github.com/ezyang/htmlpurifier/commit/43f49ac9a51b81dfd07d3bc8dcfc5ec5637a5e3b) but there are no public releases that include it...

jstanden commented 11 months ago

Thanks, @netdreamer! I'll keep an eye on htmlpurifier's upstream and pin to a new release.

We should be able to catch the TypeError at the very least so the entire request doesn't return an error.

Do you have some example text (an HTML widget or an inbound HTML message) that reproduces the issue?

netdreamer commented 11 months ago

Sorry @jstanden , it's not possibile for me to provide the HTML (a lot of sensitive info inside, and I don't want to alter it to be sure that the error is still there...), but I observed exactly what's going on.

It is related to locale: UnitConverter->convert() receives a $length variable populated with "1200 px". With our locale (it_IT.UTF-8), $n is set as 0,000000 (please note the comma instead of the digit as a decimal separator), that is invalid and abs($n) crashes.

But, I also found a different workaround that doesn't involve altering library code. HTMLPurifier uses different code segments when PHP bcmath extension is installed. With bcmath installed, issue is not present! I suppose that there is something that parses the HTML "before" in a different way that fixes the differences in locale...

jstanden commented 8 months ago

We included this fix in the 10.4.6 update last month: https://cerb.ai/releases/10.4.6/