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

Banners application crashes #42841

Closed
counterpoint opened this issue Feb 20, 2024 · 16 comments
Closed

Banners application crashes #42841

counterpoint opened this issue Feb 20, 2024 · 16 comments

Comments

@counterpoint
Copy link

Steps to reproduce the issue

Create a Joomla 5.0.2 site. Go to admin interface. Select component Banners. Exact result may vary. On Debian 12 with PHP 8.2 the site crashes and records an error "AH01067: Failed to read FastCGI header".

Expected result

Displays banners information

Actual result

Site crashes

System information (as much as possible)

Debian 12 PHP-FPM 8.2 Joomla 5.0.2

Additional comments

This can also affect third party software that calls the relevant method.

Line 42 sets $date:
$date = Factory::getDate();
This is then used in a bind to a SQL statement. But it is an object. It works if changed to:
$date = (string) Factory::getDate();

@webchun
Copy link

webchun commented Feb 22, 2024

I experience the same issue. When I publish a banner module or even access this page : administrator/index.php?option=com_banners&view=banners with Joomla installed on PHP 8.2.4, it crashes the website. Not only with Joomla 5, but I also experience the issue with Joomla 4.4.0.


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/42841.

@counterpoint
Copy link
Author

Sorry, forgot to mention that the file referred to that contains the error is .../administrator/components/com_banners/src/Helper/BannersHelper.php


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/42841.

@alikon alikon added the PHP 8.x PHP 8.x deprecated issues label Feb 22, 2024
@alikon
Copy link
Contributor

alikon commented Feb 22, 2024

unable to replicate with php 8.2.16

@Quy
Copy link
Contributor

Quy commented Feb 22, 2024

I cannot reproduce either. Please enable debug mode and set error reporting to maximum to get a stack trace.

@alikon
Copy link
Contributor

alikon commented Feb 22, 2024

from my old memories could help
https://serverpilot.io/docs/error-failed-to-read-fastcgi-header/

@webchun
Copy link

webchun commented Feb 22, 2024

I have set the error reporting to maximum and enabled the debug mode, but no stack trace shows up. The issue disappeared when I commented out this line in mod_banners.php: BannersComponentHelper::updateReset();


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/42841.

@joomdonation
Copy link
Contributor

I cannot reproduce the issue, too. However, I can see that the code is wrong. @webchun @counterpoint Could you please modify this line of code https://github.com/joomla/joomla-cms/blob/4.4-dev/administrator/components/com_banners/src/Helper/BannersHelper.php#L42 to:

$date = Factory::getDate()->toSql();

Then check it again to see if the error is gon?

@counterpoint
Copy link
Author

counterpoint commented Feb 23, 2024 via email

@counterpoint
Copy link
Author

Debug and error reporting maximum were already enabled. But since the problem causes PHP to crash, diagnostics of that kind are not feasible. That made the problem difficult to track, since the only initial information was that there was a problem somewhere - probably in PHP code. PHP crashes may well be specific to a precise environnment. But Debian 12 is pretty mainstream. This link provides similar information to the link provided by @alikon but is not specific to Wordpress and indicates that a PHP error is the most common of the possible causes - https://bobcares.com/blog/apache-error-failed-to-read-fastcgi-header/


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/42841.

@joomdonation
Copy link
Contributor

@counterpoint You are right. I just checked the code more and see that (string) $date and $date->toSql() result in same value. However, I would use $date->toSql() to make it consistent with other places in Joomla. For example https://github.com/joomla/joomla-cms/blob/4.4-dev/components/com_content/src/Model/ArticleModel.php#L184 and https://github.com/joomla/joomla-cms/blob/4.4-dev/components/com_content/src/Model/ArticleModel.php#L202 (Who knows if we use a DB driver using a date format different with Y-m-d H:i:s)

Do you want to make PR with the suggested change?

@counterpoint
Copy link
Author

That makes sense @joomdonation, although (string) is a lot more efficient. What is PR?


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/42841.

@joomdonation
Copy link
Contributor

@counterpoint In short, you make code change to to fix the issue. Please try to follow instructions here https://docs.joomla.org/Using_the_Github_UI_to_Make_Pull_Requests to make the PR

The file you need to make change is https://github.com/joomla/joomla-cms/blob/4.4-dev/administrator/components/com_banners/src/Helper/BannersHelper.php (for 4.4-dev branch and later it will be up-merge to 5.1)

@counterpoint
Copy link
Author

Thanks @joomdonation. Sorry, but I'm not familiar with making git pull requests and do not have time to find out. I've precisely identified the fault, that should be enough.


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/42841.

@joomdonation
Copy link
Contributor

No worry @counterpoint . For the time being, you can make the propose change to your site to have the issue fixed. I will make the pull request on tomorrow to fix this issue so that it won't happen again when you update to future releases of Joomla

@counterpoint
Copy link
Author

Thanks @joomdonation that's great.


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/42841.

@joomdonation
Copy link
Contributor

PR #42869 should address this issue. Please test it if you can. Thanks !

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

No branches or pull requests

6 participants