r42663 - Code Review

From MediaWiki.org

Jump to: navigation, search
Repository:MediaWiki
Revision:r42662 | r42663 (on ViewVC) | r42664 >
Date:17:56, 27 October 2008
Author:brion
Status:ok
Tags:
Comment:* (bug 11686) Make #time work with pre-1970 dates
Uses DateTime class in PHP 5.2+ to support dates outside the 1970-2038 range. On earlier versions will still fall back to strtotime with the old 32-bit Unix timestamp range limitations.
Patch by rememberthedot -- https://bugzilla.wikimedia.org/attachment.cgi?id=5416

Added a couple quick parser test cases to confirm the new behavior. However I am seeing some annoyances with how input time zones are handled, so we'll want to clean that up at some point. :)
Modified paths:

Diff [purge]

Index: trunk/extensions/ParserFunctions/ParserFunctions.php
===================================================================
--- trunk/extensions/ParserFunctions/ParserFunctions.php	(revision 42662)
+++ trunk/extensions/ParserFunctions/ParserFunctions.php	(revision 42663)
@@ -17,6 +17,8 @@
 $wgExtensionMessagesFiles['ParserFunctions'] = dirname(__FILE__) . '/ParserFunctions.i18n.php';
 $wgHooks['LanguageGetMagic'][]       = 'wfParserFunctionsLanguageGetMagic';
 
+$wgParserTestFiles[] = dirname( __FILE__ ) . "/funcsParserTests.txt";
+
 class ExtParserFunctions {
 	var $mExprParser;
 	var $mTimeCache = array();
@@ -396,22 +398,44 @@
 		if ( isset( $this->mTimeCache[$format][$date][$local] ) ) {
 			return $this->mTimeCache[$format][$date][$local];
 		}
-
-		if ( $date !== '' ) {
-			$unix = @strtotime( $date );
-		} else {
-			$unix = time();
-		}
-
-		if ( $unix == -1 || $unix == false ) {
-			wfLoadExtensionMessages( 'ParserFunctions' );
-			$result = '<strong class="error">' . wfMsgForContent( 'pfunc_time_error' ) . '</strong>';
-		} else {
-			$this->mTimeChars += strlen( $format );
-			if ( $this->mTimeChars > $this->mMaxTimeChars ) {
-				wfLoadExtensionMessages( 'ParserFunctions' );
-				return '<strong class="error">' . wfMsgForContent( 'pfunc_time_too_long' ) . '</strong>';
+		
+		#compute the timestamp string $ts
+		#PHP >= 5.2 can handle dates before 1970 or after 2038 using the DateTime object
+		#PHP < 5.2 is limited to dates between 1970 and 2038
+		
+		$invalidTime = false;
+		
+		if ( class_exists( 'DateTime' ) ) { #PHP >= 5.2
+			try { #the DateTime constructor must be used because it throws exceptions when errors occur, whereas date_create appears to just output a warning that can't really be detected from within the code
+				if ( $date !== '' ) {
+					$dateObject = new DateTime( $date );
+				} else {
+					$dateObject = new DateTime(); #use current date and time
+				}
+				
+				if ( $local ) {
+					if ( isset( $wgLocaltimezone ) ) { #convert to MediaWiki local timezone if set
+						$dateObject->setTimeZone( new DateTimeZone( $wgLocaltimezone ) );
+					} #otherwise leave in PHP default
+				} else {
+					#if local time was not requested, convert to UTC
+					$dateObject->setTimeZone( new DateTimeZone( 'UTC' ) );
+				}
+				
+				$ts = $dateObject->format( 'YmdHis' );
+			} catch (Exception $ex) {
+				$invalidTime = true;
+			}
+		} else { #PHP < 5.2
+			if ( $date !== '' ) {
+				$unix = @strtotime( $date );
 			} else {
+				$unix = time();
+			}
+			
+			if ( $unix == -1 || $unix == false ) {
+				$invalidTime = true;
+			} else {
 				if ( $local ) {
 					# Use the time zone
 					if ( isset( $wgLocaltimezone ) ) {
@@ -427,6 +451,20 @@
 				} else {
 					$ts = wfTimestamp( TS_MW, $unix );
 				}
+			}
+		}
+		
+		#format the timestamp and return the result
+		if ( $invalidTime ) {
+			wfLoadExtensionMessages( 'ParserFunctions' );
+			$result = '<strong class="error">' . wfMsgForContent( 'pfunc_time_error' ) . '</strong>';
+		} else {
+			$this->mTimeChars += strlen( $format );
+			if ( $this->mTimeChars > $this->mMaxTimeChars ) {
+				wfLoadExtensionMessages( 'ParserFunctions' );
+				return '<strong class="error">' . wfMsgForContent( 'pfunc_time_too_long' ) . '</strong>';
+			} else {
+				
 				if ( method_exists( $wgContLang, 'sprintfDate' ) ) {
 					$result = $wgContLang->sprintfDate( $format, $ts );
 				} else {
Index: trunk/extensions/ParserFunctions/funcsParserTests.txt
===================================================================
--- trunk/extensions/ParserFunctions/funcsParserTests.txt	(revision 0)
+++ trunk/extensions/ParserFunctions/funcsParserTests.txt	(revision 42663)
@@ -0,0 +1,43 @@
+# Force the test runner to ensure the extension is loaded
+# fixme... this doesn't seem to work :D
+#!! functionhooks
+#time
+#!! endfunctionhooks
+
+# fixme: #time seems to be accepting input as local time, which strikes me as wrong
+
+!! test
+Input times should probably be UTC, not local time
+!! input
+{{#time:c|15 January 2001}}
+!!result
+<p>2001-01-15T00:00:00+00:00
+</p>
+!! end
+
+!! test
+Time test in traditional range...
+!! input
+{{#time:Y|15 January 2001}}
+!! result
+<p>2001
+</p>
+!! end
+
+!! test
+Time test prior to 1970 Unix creation myth
+!! input
+{{#time:Y|5 April 1967}}
+!! result
+<p>1967
+</p>
+!! end
+
+!! test
+Time test after the 2038 32-bit Apocalype
+!! input
+{{#time:Y|28 July 2061}}
+!! result
+<p>2061
+</p>
+!! end

Property changes on: trunk/extensions/ParserFunctions/funcsParserTests.txt
___________________________________________________________________
Name: svn:eol-style
   + native

Views
Toolbox