Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Conversation widget sometimes render empty (Ajax failure) #1746

Closed
netdreamer opened this issue Oct 27, 2023 · 5 comments
Closed

Conversation widget sometimes render empty (Ajax failure) #1746

netdreamer opened this issue Oct 27, 2023 · 5 comments
Labels
bug dependencies Pull requests that update a dependency file
Milestone

Comments

@netdreamer
Copy link

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
Copy link
Author

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

@netdreamer
Copy link
Author

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

@jstanden
Copy link
Owner

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
Copy link
Author

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
Copy link
Owner

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

@jstanden jstanden added this to the Maintenance milestone Jan 12, 2024
@jstanden jstanden added bug dependencies Pull requests that update a dependency file labels Jan 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug dependencies Pull requests that update a dependency file
Projects
None yet
Development

No branches or pull requests

2 participants