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

htmlpurifier v4.18.0 is causing errors in the log #3017

Open
Grotax opened this issue Dec 31, 2024 · 9 comments
Open

htmlpurifier v4.18.0 is causing errors in the log #3017

Grotax opened this issue Dec 31, 2024 · 9 comments
Labels

Comments

@Grotax
Copy link
Member

Grotax commented Dec 31, 2024

Original issue: #2883

Downgrade: #2924

Issue at htmlpurifier ezyang/htmlpurifier#428

This needs some investigation why this version causes errors, maybe our configuration is no longer working?

@blizzz
Copy link
Member

blizzz commented Jan 17, 2025

I tried to repro the issue:

  1. composer require ezyang/htmlpurifier which pulls in 4.18
  2. add a feed
  3. force-run the background job
  4. check logs
  5. repeat steps 2-5 a couple of times

Should I encounter the problem?

Also possible: dependency hell, by getting in conflict with shipped versions in other apps. Using php-scoper to integrate it into news own namespace could be a way, if this really is the case.

@blizzz
Copy link
Member

blizzz commented Jan 31, 2025

@FadeFx can you say whether my repro steps were reasonable? And do you recall which others apps were enabled at that time?

@FadeFx
Copy link

FadeFx commented Jan 31, 2025

Actually @Grotax would be the better person to ask. I had only captured the error message and reported it.

@Grotax
Copy link
Member Author

Grotax commented Jan 31, 2025

We probably ran in a conflict with the mail app, it also uses htmlpurifier. I just checked on my instance there it has 4.18 installed now so I guess using news with 4.18 would be fine as well but maybe we should try to prevent future conflicts.

@FadeFx did/do you also happen to have the mail app installed?

@blizzz
Copy link
Member

blizzz commented Jan 31, 2025

We probably ran in a conflict with the mail app, it also uses htmlpurifier. I just checked on my instance there it has 4.18 installed now so I guess using news with 4.18 would be fine as well but maybe we should try to prevent future conflicts.

Best then indeed to go and pull in the dependencies to avoid such conflicts. Even if it is sort of aligned between apps, you cannot be sure that someone does not run a combination of different versions, shipping different things.

@FadeFx
Copy link

FadeFx commented Feb 1, 2025

@FadeFx did/do you also happen to have the mail app installed?

Yes, mail app is installed.

@blizzz
Copy link
Member

blizzz commented Feb 2, 2025

I can look into scoping the dependencies, but am without laptop the next week. Otherwise, this is what i did elsewhere in the past.

PHP 8.4 support required more adjustments though.

@Grotax
Copy link
Member Author

Grotax commented Feb 16, 2025

Hey @blizzz yea that would be nice I currently don't have much time :)

@blizzz
Copy link
Member

blizzz commented Feb 20, 2025

👍 Busy with baking cakes these weeks` nights 🫣

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants