htmlpurifier icon indicating copy to clipboard operation
htmlpurifier copied to clipboard

Update DOMLex.php

Open amanpatel opened this issue 4 years ago • 6 comments

Fixed two issues where property references that don't exist cause errors on PHP 8 (on docker setup by "tiredofit").

amanpatel avatar May 20 '21 18:05 amanpatel

By any chance do you know where in the libxml changelog DOM was modified in this way? That would help me assess this change.

ezyang avatar May 21 '21 21:05 ezyang

Could this be a PHP 8 issue? Normally undefined properties are notice level.

But for some reason in my setup for freescount (running under Docker/PHP 8), the undefined properties are triggering a ErrorExceptions. Perhaps laravel framework sets up error reporting this way. I am a bit confused as well.

Would sharing the HTML string for messages that trigger this help you recreate this error?

amanpatel avatar May 21 '21 21:05 amanpatel

@ezyang I am going to close this PR, it turns out this might be what I'm looking for: https://stackoverflow.com/questions/18497788/laravel-breaks-entire-app-on-php-notices

amanpatel avatar May 21 '21 21:05 amanpatel

reopening because we ought to be notice clean

ezyang avatar May 23 '21 00:05 ezyang

I tried to run the tests locally, but failed (Wanted to see if I could create a failing test case for you). Failing that though, I can recreate this under my envrionment. So I'm attaching the input that causes the error. Also the error log is as below:

[2021-05-22 19:43:55] production.ERROR: Undefined property: DOMAttr::$name 
(View: /www/html/resources/views/conversations/partials/thread.blade.php) 
(View: /www/html/resources/views/conversations/partials/thread.blade.php) 
(View: /www/html/resources/views/conversations/partials/thread.blade.php) 


{"userId":1,"email":"[email protected]", "exception":"[object] (ErrorException(code: 0): Undefined property: DOMAttr::$name 
(View: /www/html/resources/views/conversations/partials/thread.blade.php) 
(View: /www/html/resources/views/conversations/partials/thread.blade.php) 
(View: /www/html/resources/views/conversations/partials/thread.blade.php) at /www/html/vendor/ezyang/htmlpurifier/library/HTMLPurifier/Lexer/DOMLex.php:267, 

ErrorException(code: 0): Undefined property: DOMAttr::$name 
(View: /www/html/resources/views/conversations/partials/thread.blade.php) 
(View: /www/html/resources/views/conversations/partials/thread.blade.php) at /www/html/vendor/ezyang/htmlpurifier/library/HTMLPurifier/Lexer/DOMLex.php:267, 

ErrorException(code: 0): Undefined property: DOMAttr::$name 
(View: /www/html/resources/views/conversations/partials/thread.blade.php) at /www/html/vendor/ezyang/htmlpurifier/library/HTMLPurifier/Lexer/DOMLex.php:267, 

ErrorException(code: 0): Undefined property: DOMAttr::$name at /www/html/vendor/ezyang/htmlpurifier/library/HTMLPurifier/Lexer/DOMLex.php:267)

Fair warning (don't click on any links in the attached file). From that rabbit hole of following the function calls, I was able to determine that most likely the purifier makes its entry over here:

https://github.com/freescout-helpdesk/freescout/blob/f3aa63aa13f0e7964aff0df9e77b72b02dd0d51c/vendor/mews/purifier/src/Purifier.php#L263

html-purifier-domlex-error.txt

amanpatel avatar May 23 '21 01:05 amanpatel

@ezyang This seems to be very specific to my environment. When I checked out this repository and ran the purify function with the basic config & the input attached above, it did not display any notices or error exceptions.

I will try to reproduce in a clean php 8 docker container to see if I get the same issue. If that is true, I'll have a reproducible test case for you.

amanpatel avatar May 24 '21 03:05 amanpatel