Page 1 of 2

[9961] Possible Bug in controller.php + Fix

Posted: Wed Jan 23, 2008 6:18 pm
by StarShaper
Some people have a strange problem when they create a new article. It seems that the finishing end time is set to 1970 by default.

http://forum.joomla.org/index.php/topic,256131.0.html

Lets take a look into the code administrator\components\com_content\controller.php.

Line 384 says:

Code: Select all

		if ($id)
		{
			$row->checkout($user->get('id'));

			if (trim($row->images)) {
				$row->images = explode("\n", $row->images);
			} else {
				$row->images = array ();
			}

			$query = 'SELECT name' .
					' FROM #__users'.
					' WHERE id = '. (int) $row->created_by;
			$db->setQuery($query);
			$row->creator = $db->loadResult();

			// test to reduce unneeded query
			if ($row->created_by == $row->modified_by) {
				$row->modifier = $row->creator;
			} else {
				$query = 'SELECT name' .
						' FROM #__users' .
						' WHERE id = '. (int) $row->modified_by;
				$db->setQuery($query);
				$row->modifier = $db->loadResult();
			}

			$query = 'SELECT COUNT(content_id)' .
					' FROM #__content_frontpage' .
					' WHERE content_id = '. (int) $row->id;
			$db->setQuery($query);
			$row->frontpage = $db->loadResult();
			if (!$row->frontpage) {
				$row->frontpage = 0;
			}
		}
		else
		{
			if (!$sectionid && JRequest::getInt('filter_sectionid')) {
				$sectionid =JRequest::getInt('filter_sectionid');
			}

			if (JRequest::getInt('catid'))
			{
				$row->catid	 = JRequest::getInt('catid');
				$category 	 = & JTable::getInstance('category');
				$category->load($row->catid);
				$sectionid = $category->section;
			} else {
				$row->catid = NULL;
			}
			jimport('joomla.utilities.date');
			$createdate = new JDate();
			$row->sectionid = $sectionid;
			$row->version = 0;
			$row->state = 1;
			$row->ordering = 0;
			$row->images = array ();
			$row->publish_up = $createdate->toUnix();
			$row->publish_down = JText::_('Never');
			$row->creator = '';
			$row->created = $createdate->toUnix();
			$row->modified = $nullDate;
			$row->modifier = '';
			$row->frontpage = 0;

		}
As you can see if the article does not exist, $row->publish_down is set to Never.

Line 559 says:

Code: Select all

		if (JHTML::_('date', $row->publish_down, '%Y') <= 1969 || $row->publish_down == $db->getNullDate()) {
			$form->set('publish_down', JText::_('Never'));
		} else {
			$form->set('publish_down', JHTML::_('date', $row->publish_down, '%Y-%m-%d %H:%M:%S'));
		}
The field is set to Never, if the date is below 1970 or the date is 0000-00-00 00:00:00, which means it never expires.

At this time $row->publish_down is "Never" if the article does not exist. The first line seems to work on most systems even if there are no compatible types, but maybe not everywhere.

This could be a reason for this problem. I think the following addition should fix this problem.

Code: Select all

		if (JHTML::_('date', $row->publish_down, '%Y') <= 1969 || $row->publish_down == $nullDate /* method call not necessary */ || strcmp($row->publish_down, JText::_('Never')) == 0) {
			$form->set('publish_down', JText::_('Never'));
		} else {
			$form->set('publish_down', JHTML::_('date', $row->publish_down, '%Y-%m-%d %H:%M:%S'));
		}
A better way would be to place this if statement directly at the first place.

Re: [9961] Possible Bug in controller.php + Fix

Posted: Thu Jan 24, 2008 10:41 am
by infograf768
As I can't replicate the issue, I can't really test.
I tested the patch though and it looks like not breaking anything on current working settings.

Re: [9961] Possible Bug in controller.php + Fix

Posted: Thu Jan 24, 2008 11:44 am
by StarShaper
infograf768 wrote: As I can't replicate the issue, I can't really test.
I tested the patch though and it looks like not breaking anything on current working settings.
I'm currently aware of 4 people who have this problem with the stable 1.5 version. Unfortunately its really hard to communicate with them, because I have to wait days for a simple reply.

I cant replicate this issue on my system, too. All I know is that it might has something to do with the first statement. But I know that the actual original code is not the best.

Since $row->publish_down is set automatically to JText::_('Never'), if an article does not exist, it does does not make sense to check this again in line 559 with the actual statment.

Its true that

Code: Select all

JHTML::_('date', $row->publish_down, '%Y') <= 1969
works, because JHTML::_('date', $row->publish_down, '%Y') returns nothing (stringtype) while $row->publish_down contains the string "Never" if this article is a new one. So this statement is indeed true in PHP. This is the reason why it actually works on most systems. But it seems not to work on all systems.

However its not a good coding practice to do it that way. Comparing incompatible types is dangerous. I would refactor this code part completely, move $row->publish_down to another place.

This would bring more clarity, less code and it would work on 100% all systems.

Moreover the method call

Code: Select all

$db->getNullDate()
is not necessary, since the first line in editContent says

Code: Select all

$nullDate		= $db->getNullDate();
So we can use the variable $nullDate directly.

Re: [9961] Possible Bug in controller.php + Fix

Posted: Thu Jan 24, 2008 12:19 pm
by mcsmom
Until we can figure out how to replicate, it's impossible to know. It is very frustrating.

Re: [9961] Possible Bug in controller.php + Fix

Posted: Thu Jan 24, 2008 2:21 pm
by ot2sen
Will test in some minutes  ;)

Re: [9961] Possible Bug in controller.php + Fix

Posted: Thu Jan 24, 2008 2:30 pm
by ot2sen
Can confirm that this simple fix in line 559 did the trick.
Now Finish publishing says 'Never' instead of the former 1970...

Thanks  :)

Re: [9961] Possible Bug in controller.php + Fix

Posted: Thu Jan 24, 2008 2:36 pm
by StarShaper
Hi,

finally I was able to contact a person who has this problem and he applied the patch. It works!

However, the reason seems to be, that some systems returns another result than others. This is the line:

Code: Select all

if (JHTML::_('date', $row->publish_down, '%Y') <= 1969 || 
When you create a new article $row->publish_down contains the string "Never".

On most systems the left site returns an empty string when you call JDate with a string. This works (weak type checking), even they are incompatible types, string vs. int.

On some systems this call of JDate however returns a valid date! (1970-01-01 01:59:59)

So the code fails and you get the wrong result.

I think the code patch is simple and as it is on the right side, its the first statement which is evaluated.

If $row->publish_down contains "Never" than we can set publish_down to Never.
ot2sen wrote: Can confirm that this simple fix in line 559 did the trick.
Now Finish publishing says 'Never' instead of the former 1970...

Thanks  :)
Can you post some system information, please?  :)

Re: [9961] Possible Bug in controller.php + Fix

Posted: Thu Jan 24, 2008 2:56 pm
by ot2sen
StarShaper wrote: Can you post some system information, please?  :)
Linux
DB: 4.1.22-standard
utf8_general_ci
PHP: 4.4.4
Server: Apache/1.3.37 (Unix) mod_fastcgi/2.4.2 mod_auth_passthrough/1.8 mod_log_bytes/1.2 mod_bwlimited/1.4 PHP/4.4.4 FrontPage/5.0.2.2635.SR1.2 mod_ssl/2.8.28 OpenSSL/0.9.7a
Joomla!: Joomla! 1.5.0 Production/Stable [ Khepri ] 21-January-2008 23:55 GMT

Re: [9961] Possible Bug in controller.php + Fix

Posted: Thu Jan 24, 2008 3:15 pm
by StarShaper
@ot2sen:

Can you test what happens if you delete all content (date or Never) in the field "Finish Publishing" and try to save it? 

Here are some other configurations. All have the date problem:

Code: Select all

PHP built On: Linux h832579 2.6.9-023stab044.16-enterprise #1 SMP Fri Nov 23 20:56:09 MSK 2007 i686
Datenbank-Version: 4.1.10a
Datenbank-Kodierung: utf8_general_ci
PHP-Version: 4.3.10
Webserver: Apache/2.0.53 (Linux/SUSE)
PHP-Anbindung an den Webserver: apache2handler
Joomla!-Version: Joomla! 1.5.0 Production/Stable [ Khepri ] 21-January-2008 23:55 GMT

Code: Select all

PHP 4.4.7
Apache 2

Re: [9961] Possible Bug in controller.php + Fix

Posted: Thu Jan 24, 2008 3:26 pm
by Peter Clements
I can confirm that a fresh install of Joomla! 1.5.0 Stable on our server also saw new content items with a 'Finish Publishing' date of 1970 and that modifying line 559 of administrator/components/com_content/controller.php fixed the issue, it now says "Never" for new articles.

System Info:
PHP Built on:  Linux Oulton 2.6.9-023stab044.11 #1 Sun Sep 30 11:49:45 MSD 2007 i686
Database Version: 4.1.20
Database Collation: utf8_general_ci
PHP Version: 5.0.4
Web Server: Apache/2.0.54 (Fedora)
Web Server to PHP interface: apache2handler
Joomla! Version: Joomla! 1.5.0 Production/Stable [ Khepri ] 21-January-2008 23:55 GMT
Please request any further info from me if you think it might help!

Re: [9961] Possible Bug in controller.php + Fix

Posted: Thu Jan 24, 2008 3:28 pm
by ot2sen
StarShaper wrote: Can you test what happens if you delete all content (date or Never) in the field "Finish Publishing" and try to save it? 
Still works fine. Tried in 'New' and in 'Edit' mode. Having cleaned field, then it still adds 'Never on 'Save'

Re: [9961] Possible Bug in controller.php + Fix

Posted: Thu Jan 24, 2008 3:31 pm
by Peter Clements
StarShaper wrote:Can you test what happens if you delete all content (date or Never) in the field "Finish Publishing" and try to save it?
Have just tried this myself, all is fine, if I clear out the field then it becomes "Never" upon saving and re-opening the article.

Re: [9961] Possible Bug in controller.php + Fix

Posted: Thu Jan 24, 2008 4:13 pm
by StarShaper
Hi @all!  :)

Its very interesting that all people seems to use PHP 4. Never saw someone who has this problem and uses PHP 5. But there are also PHP 4 users without this bug.

However, PHP 5 users are pretty lucky because it returns the right result due to the weak type checking in PHP. There are other lines like 638:

Code: Select all

		// Handle never unpublish date
		if (trim($row->publish_down) == JText::_('Never') || trim( $row->publish_down ) == '')
		{
			$row->publish_down = $nullDate;
		}
		else
		{
			if (strlen(trim( $row->publish_down )) <= 10) {
				$row->publish_down .= ' 00:00:00';
			}
			$date = new JDate($row->publish_down, $tzoffset);
			$row->publish_down = $date->toMySQL();
		}
String comparison with == is not really the best way to do. For example if you save an article, you can write the number 16253749322 into the field "Finish Publishing" and it works. It sets the date in mysql to 1940-08-27 01:28:58.

It would be better to rewrite this code to make it more fail-safe and little bit smarter.

Thats the reason why all great languages have a strong type checking.  :D

Re: [9961] Possible Bug in controller.php + Fix

Posted: Thu Jan 24, 2008 4:18 pm
by mcsmom
I want to make the following observation, having done a lot of testing with php 4.4.7 and some other php 4 versions.

Some versions on PHP 4 are using a tremendous amount of memory, over the default limits of php and of some hosts.

This may be causing some of these symptoms.

You can see how much memory is being used by turning on debug.

In my experience 4.4.7 is using about 3-4 times the memory of php 5 or some other php 4 versions.

Re: [9961] Possible Bug in controller.php + Fix

Posted: Thu Jan 24, 2008 4:33 pm
by Peter Clements
StarShaper wrote:Its very interesting that all people seems to use PHP 4. Never saw someone who has this problem and uses PHP 5. But there are also PHP 4 users without this bug.
I'm using PHP 5.0.4, as per my post above.

Re: [9961] Possible Bug in controller.php + Fix

Posted: Thu Jan 24, 2008 4:38 pm
by StarShaper
Peter Clements wrote: I'm using PHP 5.0.4, as per my post above.
You are right! Missed your posting, sorry. :)

Than I don't see any reason why on some hosts the code (JDate) returns an empty string and on other host a valid date (1970).

Maybe some Joomla testers can figure this out.

Re: [9961] Possible Bug in controller.php + Fix

Posted: Thu Jan 24, 2008 4:53 pm
by greyow
on linux:

Code: Select all

if (JHTML::_('date', $row->publish_down, '%Y') <= 1969 || $row->publish_down == $db->getNullDate()) {
$db->getNullDate() = '0000-00-00 00:00:00'
$row->publish_down = 'Never' or date '2008-01-30 23:00:00'

second part of if is never equal

it works on my system:

Code: Select all

if (JHTML::_('date', $row->publish_down, '%Y') <= 1969 || JHTML::_('date', $row->publish_down, '%Y-%m-%d %H:%M:%S') == JHTML::_('date',$db->getNullDate(), '%Y-%m-%d %H:%M:%S' )) {


PHP Built on:  Linux gcube 2.6.21.4 #3 SMP Sun Sep 30 19:25:07 CEST 2007 i686
Database Version: 5.0.45-Debian_1-log
Database Collation: utf8_general_ci
PHP Version: 4.4.4-9+lenny1
Web Server: Apache/1.3.34 (Debian) PHP/4.4.4-9+lenny1 mod_fastcgi/2.4.2
Web Server to PHP interface: apache
Joomla! Version: Joomla! 1.5.0 Production/Stable [ Khepri ] 21-January-2008 23:55 GMT
User Agent: Mozilla/5.0 (X11; U; Linux i686; hu; rv:1.8.1.11) Gecko/20071127 Firefox/2.0.0.11

Re: [9961] Possible Bug in controller.php + Fix

Posted: Thu Jan 24, 2008 5:16 pm
by StarShaper
@greyow

And whats your point?  :)

Nulldate is defined in JDatabaseMySQL.

var $_nullDate = '0000-00-00 00:00:00';

So your code does not really make sense, because nullDate is alread formatted. It should be:

Code: Select all

$row->publish_down == $nullDate // nulldate already defined
EDIT: Now I think I know what you mean. But it is not true.

Code: Select all

$row->publish_down == $db->getNullDate()
This line means, that if the database entry is 0000-00-00 00:00:00 set the "Finish Publishing" field to Never. So it is absolutely correct.

And yes, of course there are cases when it is equal. Always then, when the article was already saved with no expiration.  ;)

Re: [9961] Possible Bug in controller.php + Fix

Posted: Thu Jan 24, 2008 6:01 pm
by greyow
yes

if the JHTML::_('date', gets illegal date form, it will return 1970-01-01 01:59:59

the '0000-00-00 00:00:00' (and 'Never') is not legal date form on unix system.

Re: [9961] Possible Bug in controller.php + Fix

Posted: Thu Jan 24, 2008 6:09 pm
by diri
greyow wrote: yes

if the JHTML::_('date', gets illegal date form, it will return 1970-01-01 01:59:59

the '0000-00-00 00:00:00' (and 'Never') is not legal date form on unix system.
as well as at some "unsupported" platforms.

Obey to the rules, please, and be as strict as possible in as many relations as possible when typing code.

Re: [9961] Possible Bug in controller.php + Fix

Posted: Thu Jan 24, 2008 6:33 pm
by StarShaper
greyow wrote: yes

if the JHTML::_('date', gets illegal date form, it will return 1970-01-01 01:59:59

the '0000-00-00 00:00:00' (and 'Never') is not legal date form on unix system.
That is true, but thats not the point. '0000-00-00 00:00:00' is just the MySQL date entry. And this entry is valid! Take a look at the reference:

http://dev.mysql.com/doc/refman/5.0/en/datetime.html
llegal DATETIME, DATE, or TIMESTAMP values are converted to the “zero” value of the appropriate type ('0000-00-00 00:00:00' or '0000-00-00').

Re: [9961] Possible Bug in controller.php + Fix

Posted: Thu Jan 24, 2008 6:33 pm
by infograf768
Please just try to change line 443 from

Code: Select all

$row->publish_down = JText::_('Never');
to

Code: Select all

$row->publish_down = $nullDate;

Re: [9961] Possible Bug in controller.php + Fix

Posted: Thu Jan 24, 2008 6:38 pm
by StarShaper
The fact is, the official code is:

Code: Select all

		if (JHTML::_('date', $row->publish_down, '%Y') <= 1969 || $row->publish_down == $db->getNullDate()) {
			$form->set('publish_down', JText::_('Never'));
		} else {
			$form->set('publish_down', JHTML::_('date', $row->publish_down, '%Y-%m-%d %H:%M:%S'));
		}
The problem is definitely this line:

Code: Select all

JHTML::_('date', $row->publish_down, '%Y') <= 1969
because $row->publish_down contains the string "Never" if the article is new. This is done in line 443. But the function date can't produce a valid date from the string "Never". "Never" is not a date not an int.

So it produces bogus results. On most systems this implicit type conversion results in a correct statement. However it is not correct and this is the reason, why on some platforms the code fails.

We have to check additionally if $row->publish_down is set to "Never". This is done, by comparing the variable with strcmp(). And that solves the problem.

Nothing more!  ;)

Re: [9961] Possible Bug in controller.php + Fix

Posted: Thu Jan 24, 2008 6:46 pm
by StarShaper
infograf768 wrote: Please just try to change line 443 from

Code: Select all

$row->publish_down = JText::_('Never');
to

Code: Select all

$row->publish_down = $nullDate;
This is also a possible solution, thats right!

I cant test it on my system, but it should work.

As I already said in my first posting, I would refactor the whole code. Actually it is not really logical.

Re: [9961] Possible Bug in controller.php + Fix

Posted: Thu Jan 24, 2008 10:21 pm
by kdevine
This issues has been confirmed for PHP versions < 5.1.0 and in cases when the location defined in global config cancels the server offset to GMT.

An artifact, #9412, has been added to the tracker with a patch file provided.

http://joomlacode.org/gf/project/joomla ... em_id=9412

Re: [9961] Possible Bug in controller.php + Fix

Posted: Thu Jan 24, 2008 10:42 pm
by StarShaper
kdevine wrote: This issues has been confirmed for PHP versions < 5.1.0 and in cases when the location defined in global config cancels the server offset to GMT.

An artifact, #9412, has been added to the tracker with a patch file provided.

http://joomlacode.org/gf/project/joomla ... em_id=9412
Joomlacode says: Open Date of this tracker was 2008-01-24. This was after this discussion started. ;)

However, nice to see that the reason is indeed how PHP handles a function. I thought that it might has something to do with the PHP version, but could not proof it.

Except from this issue I think your patch is a nice correction of the class JDate, but the current code in controller.php should be patched with infograf768 solution.

It does not make sense to pass a String like "Never" to date. This implicit type conversion causes often a lot of problems. Just set publish_down to nullDate and it should be ok.  :)

Re: [9961] Possible Bug in controller.php + Fix

Posted: Thu Jan 24, 2008 10:57 pm
by kdevine
StarShaper wrote: Joomlacode says: Open Date of this tracker was 2008-01-24. This was after this discussion started. ;)
We are using the tracker now for confirmed bugs. We did not open a tracker item for this issue until we knew what exactly the problem was and it could be reliably reproduced. This thread helped us do that. My post was for the moderators so they know what to do with this thread and any related ones.
StarShaper wrote: Except from this issue I think your patch is a nice correction of the class JDate, but the current code in controller.php should be patched with infograf768 solution.

It does not make sense to pass a String like "Never" to date. This implicit type conversion causes often a lot of problems. Just set publish_down to nullDate and it should be ok.  :)
For the sake of best practices I agree completely but nevertheless the bug does live in JDate.

Re: [9961] Possible Bug in controller.php + Fix

Posted: Thu Jan 24, 2008 11:03 pm
by StarShaper
kdevine wrote: For the sake of best practices I agree completely but nevertheless the bug does live in JDate.
I fully agree with you, but controller.php should be changed too.  :)

Re: [9961] Possible Bug in controller.php + Fix

Posted: Mon Feb 04, 2008 11:51 pm
by cbrody
I have this problem too. What's the recommended fix for it?

Re: [9961] Possible Bug in controller.php + Fix

Posted: Tue Feb 05, 2008 11:10 am
by infograf768
delete the string in red and add the string in blue.
Index: libraries/joomla/utilities/date.php
===================================================================
--- libraries/joomla/utilities/date.php (revision 9962)
+++ libraries/joomla/utilities/date.php (working copy)
@@ -132,7 +132,7 @@
$this->_date -= $tzOffset;
return;
}
$this->_date = strtotime($date);
$this->_date = (strtotime($date) == -1) ? false : strtotime($date);
if ($this->_date) {
$this->_date -= $tzOffset;
}
.