propelorm / sfPropelORMPlugin

symfony 1.x plugin for Propel.
http://www.propelorm.org/
Other
109 stars 95 forks source link

PropelDateTime doesn't handle unix timestamps correctly #207

Closed lathspell closed 8 years ago

lathspell commented 11 years ago

One of my model functions in model/base/BaseDslam.php seems to accept a Unix Timestamp as integer value, according to the comment:

  /**
 * Sets the value of [updated_at] column to a normalized version of the date/time value specified.
 * 
 * @param      mixed $v string, integer (timestamp), or DateTime value.
 *               Empty strings are treated as NULL.
 * @return     Dslam The current object (for fluent API support)
 */
public function setUpdatedAt($v)
{
    $dt = PropelDateTime::newInstance($v, null, 'DateTime');

The method then calls PropelDateTime::newInstance() with this value which itself creates a new PHP DateTime object.

The DateTime constructor as of PHP-5.4 accepts Unix Timestamps only if they are written as "@1372264803" and thus creates an Exception:

Exception' with message 'DateTime::__construct(): Failed to parse time string (1372264803) at position 7 (8): Unexpected character' in /usr/share/php/propel/util/PropelDateTime.php:48

This could be fixed by checking if the parameter is all-digits (is_int() does not work if its all-digits as string!):

if (preg_match('/^\d+$/', $v) { $v = '@' . $v; }

lathspell commented 11 years ago

Wait, it maybe I have several PropelDateTime.php versions on my system, the Symfony plugin and the PEAR package, but even the more recent one still uses is_numeric() which does not work if the timestamp comes as integer e.g. with setUpdateAt(123456789):

public static function newInstance($value, DateTimeZone $timeZone = null, $dateTimeClass = 'DateTime')
...
    if (self::isTimestamp($value)) { // if it's a unix timestamp
....
public static function isTimestamp($value)
{
    if (!is_numeric($value)) {
          return false;
rozwell commented 8 years ago

Firstly, that's a Propel, not the sfPropelORMPlugin bug: sfPropelORMPlugin/lib/vendor/propel/runtime/lib/util/PropelDateTime.php

Secondly, why wouldn't it work with integers? Method isTimestamp() seems to be working fine with the example you provided on PHP5.5 and PHP7.0.

I can see the issues with integers on 32bit machines, but you'd have to go beyond year 2038...