Bug in Payplans breaks RSS feeds (and such as PDF)
By Weeblr on Tuesday, 01 October 2019
Posted in General Issues
Replies 10
Likes 0
Views 1K
Votes 0
Hi

I have just realized that Payplans (latest 4.x) is breaking the RSS feeds on my sites. I did some debugging and quickly found that it's a well known issue that affected multiple extensions over the years.

The problem happens because Payplans calls JFactory:getDocument() before the onAfterRoute event. In doing so, it forces Joomla to have a HTML document type and so the RSS feeds are broken.

Very importantly, this will not happen on all sites. It happens on ours because our RSS feeds have URLs such as /blog/feed/rss/

On site with standard Joomla SEF, feeds have URLs such as: /blog/feed/rss/?format=feed. When there's a "?format=feed" in the URL, the problem does not occur.

Note that the same will happen with any non-html request (ie pdf, json) as long as "?format=xxx" is not in the URL.

The bug is caused in 2 locations:

Your system plugin extends the class PPPlugins. In its constructor, this class does:

$this->info = PP::info();
$this->theme = PP::themes();


PP:info() cause the loading of class PayPlans which has $this->doc = JFactory::getDocument(); in its constructor (line 98 of administrator/components/com_payplans/includes/dependencies.php)

and

PP::themes() also has $this->doc = JFactory::getDocument(); in its constructor (line 39 of administrator/components/com_payplans/includes/themes/themes.php)

Both these calls (JFactory::getDocument()) are not allowed in Joomla before onAfterRoute as they cause the bug we are seeing.

It is not legal to call JFactory::getDocument() in the constructor of a system plugin before onAfterRoute has been triggered.
The reason is that before onAfterRoute, the SEF URL has not been decoded yet and so Joomla does not know which format is the document. It therefore forces it to html. That's ok in general but causes a bug for non-html document such as rss feeds, json or pdf.

I have not yet had time to look for a fix myself but it's actually urgent. It appears our RSS feeds are broken due to this issue since we updated to Payplans 4.

Best regards
It seems like there is no proper way to address this apart from modifying the plugins from inheriting from those objects. Try to download the attached file and upload it into /plugins/system/payplans/
Mark
·
Tuesday, 01 October 2019 11:33
·
0 Likes
·
0 Votes
·
0 Comments
·
Hi Mark,

Thanks for that. It tried but it does not work yet. It only removed one way of triggering the error. The system plugin still runs some code in the onAfterInitialize event that loads all your plugins(line 25 of administrator/components/com_payplans/includes/dispatcher/dispatcher.php).
As all plugins extend PPPlugin - which is expected - PPPlugins is instanciated and in its constructor you still have 



$this->info = PP::info();

$this->theme = PP::themes();


Both of them eventually uses the Payplans class and that class has 



$this->doc = JFactory::getDocument();


in its constructor. So the error is still there.

I can only think of 2 ways out of this:

1 - Do you need to run "onPayplansSystemStart" at Joomla's onAfterInitialize? can it wait until onAfterRoute?

I2 - If not possible, what can be done is in class Payplans you use a method to load $this->doc. That way, the document is only retrieved from Joomla when it's needed, not in the constructor.

Instead of having in the constructor:



$this->doc = JFactory::getDocument();


You could have a getDoc() method:



protected $jdoc = null;



public function getDoc() {

if(empty($this->jdoc)) {

$this->jdoc = JFactory::getDocument();

}

return $this->jdoc;

}


However in all your code you need to replace instances of $this->doc with $this->getDoc() which maybe trouble.

Another way to do this may be to remove the "public $doc" property and use a magic get method? that way you don't have to change your code?

Best regards
Weeblr
·
Tuesday, 01 October 2019 15:19
·
0 Likes
·
0 Votes
·
0 Comments
·
Hi again,

So I have worked on a local copy and I got it to work doing the following:

1 - administrator/components/com_payplans/includes/dependencies.php:

- remove the $doc property
- remove $this->doc = JFactory::getDocument(); in constructor
- add magic method:



public function __get($property)

	{

		static $doc = null;



		switch ($property)

		{

			case 'doc':

				if (empty($doc))

				{

					$doc = JFactory::getDocument();

				}

				return $doc;

				break;

		}

	}


2 - administrator/components/com_payplans/includes/themes/themes.php

Exact same thing.

By making sure the "doc" property is only initialized when it's needed, we avoid that problem. I have not ran many tests yet but at least it solves the RSS feeds issue. I'll port that to our live sites now and monitor for errors.

Best regards
Weeblr
·
Tuesday, 01 October 2019 15:41
·
0 Likes
·
0 Votes
·
0 Comments
·
Hi

Note that by doing the above, the Payplans system plugin does not need to be modified. It can still extends PPPlugin.

Best regards
Weeblr
·
Tuesday, 01 October 2019 15:48
·
0 Likes
·
0 Votes
·
0 Comments
·
Hey Yannick,

Thanks, will look into this and investigate further
Mark
·
Wednesday, 02 October 2019 16:36
·
0 Likes
·
0 Votes
·
0 Comments
·
Hi Mark,

I just installed Payplans 4.2.2 and found that the bug has not been fixed totally.

In administrator/components/com_payplans/includes/themes/themes.php

you have not applied:

- remove $this->doc = JFactory::getDocument(); in constructor


So the problem occurs the same. Commenting or removing that line solves the problem.

Best regards
Yannick Gaultier
weeblr.com / @weeblr
Weeblr
·
Wednesday, 14 October 2020 17:24
·
0 Likes
·
0 Votes
·
0 Comments
·
Hi

As it turns out, the changes you made are incorrect, they also cause a fatal error. You changed my code from:



public function __get($property)

	{

		static $doc = null;



		switch ($property)

		{

			case 'doc':

				if (empty($doc))

				{

					$doc = JFactory::getDocument();

				}

				return $doc;

				break;

		}

	}


to 



 public function __get($property)

    	{

    		if ($property == 'doc') {

    			static $doc = null;

    

    			if (is_null($doc)) {

    				$doc = JFactory::getDocument();

                               return $doc;

    			}

       		}

    	}


but that's not equivalent. The 'return $doc' is only executed on the first call. Subsequent calls cause a fatal error.

Please just use the provided code. Or you can change it to:



public function __get($property)

    	{

    		if ($property == 'doc') {

    			static $doc = null;

    

    			if (is_null($doc)) {

    				$doc = JFactory::getDocument();

    			}

    

    			// weeblr

    			return $doc;

    			// weeblr

    		}

    	}


This works and is what I have running at the moment.
The similar code in /dependencies.php is working but it has 2 "return $doc" while only one is needed, if located in the right place.

Best regards
Yannick Gaultier
weeblr.com / @weeblr
Weeblr
·
Wednesday, 14 October 2020 19:33
·
0 Likes
·
0 Votes
·
0 Comments
·
Can you try with this file and see if this works for you?
Mark
·
Friday, 16 October 2020 14:40
·
0 Likes
·
0 Votes
·
0 Comments
·
Hi Mark,

yes, that file works!

Best regards
Yannick Gaultier
weeblr.com / @weeblr
Weeblr
·
Friday, 16 October 2020 15:04
·
0 Likes
·
0 Votes
·
0 Comments
·
Thanks for confirming Yannick.
Mark
·
Saturday, 17 October 2020 13:56
·
0 Likes
·
0 Votes
·
0 Comments
·
View Full Post