Zend Framework

getDateModified() returns incorrect date if RSS feed has near-standard date formatting

Details

  • Type: Bug Bug
  • Status: Resolved Resolved
  • Priority: Major Major
  • Resolution: Fixed
  • Affects Version/s: 1.9.2
  • Fix Version/s: 1.9.3
  • Component/s: Zend_Feed_Reader
  • Labels:
    None

Description

I have the following date in a RSS feed:
Sun, 11 Jan 2009 09:55:59 GMT

The getDateModified function in Zend_Feed_Reader_Entry_Rss returns a Zend_Date object corresponding to:
Sun, 01 Nov 2009 09:55:59 +0000

Expected result:
Sun, 11 Jan 2009 09:55:59 +0000

This is a month/day inversion (m-d-Y is US standard, d-m-Y is EU standard).

As it looks like php's builtin strtotime function understands/translates the date string properly, I propose to first try a strtotime conversion before attempting a Zend_Date resolution using RFC standards.

(updated 2009-09-21 16:46 UTC) Proposed working patch for Zend/Feed/Reader/Entry/Rss.php (rev 16966, Zend Framework v1.9.2):

256a257
>             
257a259,262
>             	$dateModifiedParsed = strtotime($dateModified);
>             	if ($dateModifiedParsed) {
>             		$date = new Zend_Date($dateModifiedParsed);
>             	} else {
277a283
>         }
  1. Rss.patch
    21/Sep/09 9:41 AM
    0.2 kB
    Christophe Deliens
  2. Rss.patch
    21/Sep/09 7:06 AM
    0.2 kB
    Christophe Deliens

Issue Links

Activity

Hide
Christophe Deliens added a comment -

Rss.php patch file

Show
Christophe Deliens added a comment - Rss.php patch file
Hide
Pádraic Brady added a comment -

Fixed (for given example) in r18340.

Thanks for the report! Give the fix a whirl (in trunk and release-1.9 branch) to see if it checks out for any other dates in the same feed.

Show
Pádraic Brady added a comment - Fixed (for given example) in r18340. Thanks for the report! Give the fix a whirl (in trunk and release-1.9 branch) to see if it checks out for any other dates in the same feed.
Hide
Christophe Deliens added a comment -

Patch #1 was incorrect... please use this one instead on original file.

Show
Christophe Deliens added a comment - Patch #1 was incorrect... please use this one instead on original file.
Hide
Pádraic Brady added a comment -

Issue is resolved - the example in the description can be parsed using the Zend_Date::RSS option as a second to last resort (really should be RFC 822 or 2822 per the RSS Advisory Standard). So while the patch is not applied, the alternative change does work for the given example.

Show
Pádraic Brady added a comment - Issue is resolved - the example in the description can be parsed using the Zend_Date::RSS option as a second to last resort (really should be RFC 822 or 2822 per the RSS Advisory Standard). So while the patch is not applied, the alternative change does work for the given example.
Hide
Christophe Deliens added a comment -

I don't agree Pádraic.

Look closer at the first patch: if "strtotime" can't parse, PHP will return false.
So the variable $dateModified will be FALSE and that value will tried to be parsed by the next "->set" instructions, which will end in the latest Exception catch and FAIL.

Trust me, I had the problem.

Show
Christophe Deliens added a comment - I don't agree Pádraic. Look closer at the first patch: if "strtotime" can't parse, PHP will return false. So the variable $dateModified will be FALSE and that value will tried to be parsed by the next "->set" instructions, which will end in the latest Exception catch and FAIL. Trust me, I had the problem.
Hide
Pádraic Brady added a comment -

I'm not sure you're following the logic from the start in how I resolved this. The original issue description includes one example of the problematic parsing. I added a unit test to check the bug exists, using the unit test I added the new parsing set() call, the unit test then passed. Therefore I resolved the issue as fixed. I can't add yet another parsing attempt without a reason to do so, and a unit test to support its application against a date no other step could parse. Is there another date type which requires strtotime()?

Show
Pádraic Brady added a comment - I'm not sure you're following the logic from the start in how I resolved this. The original issue description includes one example of the problematic parsing. I added a unit test to check the bug exists, using the unit test I added the new parsing set() call, the unit test then passed. Therefore I resolved the issue as fixed. I can't add yet another parsing attempt without a reason to do so, and a unit test to support its application against a date no other step could parse. Is there another date type which requires strtotime()?

People

Vote (0)
Watch (0)

Dates

  • Created:
    Updated:
    Resolved: