Bug #72878

wrong datetime handling, they are not UTC in db

Added by Oliver Pfaff over 3 years ago. Updated over 2 years ago.

Status:
Closed
Priority:
Should have
Assignee:
-
Category:
-
Target version:
-
Start date:
2016-01-21
Due date:
% Done:

0%

TYPO3 Version:
6.2
PHP Version:
5.5
Tags:
Complexity:
Is Regression:
No
Sprint Focus:

Description

Hello folks,
we have a problem with the handling of datetimes. Actually the datetime are saved with the actual timezone and not UTC.

The problem seems in the DataHandler.php

if ($isDateOrDateTimeField) {
   // Convert the timestamp back to a date/time
   $res['value'] = $res['value'] ? date($format, $res['value']) : $emptyValue;
}

with the date method we add again the timezone previously removed here:


public function checkValue_input_Eval($value, $evalArray, $is_in) {
        $res = array();
        $newValue = $value;
        $set = TRUE;
        foreach ($evalArray as $func) {
            switch ($func) {
                case 'int':

                case 'year':

                case 'time':

                case 'timesec':
                    $value = (int)$value;
                    break;
                case 'date':

                case 'datetime':
                    $value = (int)$value;
                    if ($value > 0 && !$this->dontProcessTransformations) {
                        $value -= date('Z', $value);
                    }
                    break;

so the datetime will be saved with their local timezone.

This Problem is pretty heavy because extbase expect datetime to be UTC.

protected function mapDateTime($value, $storageFormat = NULL, $targetType = 'DateTime') {
        if (empty($value) || $value === '0000-00-00' || $value === '0000-00-00 00:00:00') {
            // 0 -> NULL !!!
            return NULL;
        } elseif ($storageFormat === 'date' || $storageFormat === 'datetime') {
            // native date/datetime values are stored in UTC
            $utcTimeZone = new \DateTimeZone('UTC');
            $utcDateTime = \TYPO3\CMS\Core\Utility\GeneralUtility::makeInstance($targetType, $value, $utcTimeZone);
            $currentTimeZone = new \DateTimeZone(date_default_timezone_get());
            return $utcDateTime->setTimezone($currentTimeZone);
        } else {
            return \TYPO3\CMS\Core\Utility\GeneralUtility::makeInstance($targetType, date('c', $value));
        }
    }

I have fixed this you can see my changes here:
[[https://github.com/Xippo/TYPO3.CMS/pull/1/files]]

With this fix the datetimes are stored as UTC in the DB. The backend shows the right dates and times and Extbase give you datetimes with the right time. Before i had my timezone offset added on the DB time.

I don't have tested this in 7.6 but i think there is the same.

I have discovered another bug in Extbase, related too datetimes.
If you do somthing like that:

public function listAction() {
        $tests = $this->testRepository->findAll();
        /*
        /** @var \TEST\Test\Domain\Model\Test $object */
        $object = $tests->getFirst();

        $datetime = $object->getDatetime();
        $datetime->add(new \DateInterval('P0Y0DT0H1M'));
        $object->setDatetime($datetime);
        $this->testRepository->update($object);

        $this->view->assign('tests', $tests);
    }

Extbase will add every time the timezone offset, like you can see above in the mapDateTime function, but Extbase don't remove the offset on the way in the database so in my case the code above adds every time 1h when the list view is called.

I don't now if it is a bug but people have to now this because they have to set the datetimes on UTC by it self. Or we change the entire handling in TYPO3 and Extbase and store datetime with the local timezone but this don't seems to be a good idea.


Related issues

Related to TYPO3 Core - Bug #68849: Unstable persistence handling of DateTime (don't get what you set) Closed 2015-08-07
Related to TYPO3 Core - Bug #68651: Datetime() properties have wrong timezone Accepted 2015-07-30
Related to TYPO3 Core - Bug #64953: dbType date and datetime fields are saved in wrong timezone Closed 2015-02-10
Related to TYPO3 Core - Feature #61110: Support for timezones in all date fields in TYPO3 BE New 2014-08-21
Related to TYPO3 Core - Bug #79613: Saving wrong Date into DB if Field is of type DATE New 2017-02-03

Associated revisions

Revision e5f27f56 (diff)
Added by Andreas Wolf about 3 years ago

[TASK] Add tests for Extbase UTC date/datetime handling

The actual bug has been fixed a while ago with commit
827219a1c35b4aca6dbab5855a36e9277b2ec8f4, but the tests I wrote somehow
got lost. This patch adds them back and shows that the problem has
actually been fixed.

Change-Id: Ibca2524d25573d67b3541761c8cf2b662fcd3423
Related: #72878
Releases: master, 7.6
Reviewed-on: https://review.typo3.org/47137
Tested-by: Bamboo TYPO3com <>
Reviewed-by: Andreas Wolf <>
Tested-by: Andreas Wolf <>

Revision deee7918 (diff)
Added by Andreas Wolf about 3 years ago

[TASK] Add tests for Extbase UTC date/datetime handling

The actual bug has been fixed a while ago with commit
827219a1c35b4aca6dbab5855a36e9277b2ec8f4, but the tests I wrote somehow
got lost. This patch adds them back and shows that the problem has
actually been fixed.

Change-Id: Ibca2524d25573d67b3541761c8cf2b662fcd3423
Related: #72878
Releases: master, 7.6
(cherry picked from commit e5f27f56e1420e3d3c2dee7e0782a4c116f39779)
Reviewed-on: https://review.typo3.org/49743
Tested-by: Bamboo TYPO3com <>
Reviewed-by: Andreas Wolf <>
Tested-by: Andreas Wolf <>

History

#1 Updated by Oliver Pfaff over 3 years ago

  • Target version set to 6.2.18

#2 Updated by Wouter Wolters over 3 years ago

  • Target version deleted (6.2.18)

Please do not set the target version your self. Thanks.

#3 Updated by Andreas Wolf over 3 years ago

  • Status changed from New to Accepted

From looking at the code I’m pretty sure that the handling of date/datetime fields is correct in Extbase, but int fields (UNIX timestamps) are broken: The value is stored in UTC, but on reading it, PHP assumes it is a localized date format. Therefore, the shift would need to be done manually, as is done e.g. for FormEngine.

#4 Updated by Andreas Wolf over 3 years ago

Andreas Wolf wrote:

From looking at the code I’m pretty sure that the handling of date/datetime fields is correct in Extbase, but int fields (UNIX timestamps) are broken:

After closer investigation, it turned out to be the other way round actually. Integer fields are correctly handled, but Date/Datetime fields are currently broken in two ways. Fixes are on their way.

#5 Updated by Gerrit Code Review over 3 years ago

  • Status changed from Accepted to Under Review

Patch set 1 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/47137

#6 Updated by This Mächler over 3 years ago

How will you handle the following problem if you release the patch:
Productive systems with dateTime values stored in local time rather than UTC, maybe with hardcoded workarounds to overcome the problem. The issue is there for years now. e.g.:

    /**
     * Returns the dateTime
     *
     * @return \DateTime $dateTime
     */
    public function getDateTime() {
        if( !($dt = $this->dateTime) )
            return NULL;
        // revert timezone shift done by mapper
        $dtLoc = new \DateTime();
        return $dtLoc->setTimeStamp( $dt->getTimeStamp() - $dt->getOffset() );
    }    

Suggestion:
Make it possible to choose how TYPO3 should treat dateTime values:

1. TYPO3 assumes that dateTime values from the DB are in local time and need no conversion from/to the FE or BE. This should be the default setting to avoid breaking a running system.

2. TYPO3 assumes that dateTime values are stored in UTC, conversion takes place from/to the FE or BE.

This will not solve the problem with the already implemented workarounds. Maybe there is no solution for that.

#7 Updated by This Mächler over 3 years ago

I do absolutely do not understand, how a serious bug like this can not be solved for years..
Search for "typo3 DateTime bug" in your favorite search engine. Some of the bug reports are 2,3,4,5 years old.

#8 Updated by Christian Kuhn over 3 years ago

storing date fields as utc in db comes along with the change for #61110

#9 Updated by Sebastian Mazza about 3 years ago

Pleas do not change the behaviour of UNIX Timestamps (int columns) because the are handled correctly. Saved as UTC to DB and converted back to the local time zone after read. This works well, both at Extbase and BE forms.
In my opinion, the datetime columns should be handled in the same way as int columns with UNIX Timestamps.

#10 Updated by Viktor Livakivskyi about 3 years ago

There is a special case with DB fields of type 'date'.

Imagine a case, when a DateTime object is created in "Europe/Berlin" like this

$dateObject = new \DateTime('2016-07-13');

This will correspond to 2016-07-13 00:00:00.000000 in timezone "Europe/Berlin".

So, in case ExtBase will convert it to UTC during persistence, the date will correspond to 2016-07-12 22:00:00.000000 in UTC timezone. And saved to DB will result in "2016-07-12" stored.

Then on retrieval from DB the DateTime object will be created and Timezone shift added, which will correspond to 2016-07-12 02:00:00.000000 in "Europe/Berlin".
One day is lost then.

So, for case with native DB 'date' field the timezone shift should be totally ignored.

#11 Updated by Oliver Hader about 3 years ago

  • Parent task set to #77562

#12 Updated by Gerrit Code Review about 3 years ago

Patch set 2 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/47137

#13 Updated by Gerrit Code Review about 3 years ago

Patch set 3 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/47137

#14 Updated by Gerrit Code Review about 3 years ago

Patch set 4 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/47137

#15 Updated by Gerrit Code Review about 3 years ago

Patch set 5 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/47137

#16 Updated by Andreas Wolf about 3 years ago

Sebastian Mazza wrote:

Pleas do not change the behaviour of UNIX Timestamps (int columns) because the are handled correctly. Saved as UTC to DB and converted back to the local time zone after read. This works well, both at Extbase and BE forms.

Actually, the behaviour is different from what you described: The date is passed as UTC from the client (browser), converted to the server’s timezone (PHP’s date.timezone setting) and then saved to the database. This is a subtle difference you will only realize when using different timezones on client and server—also took me a while to find that out.

As for this issue, it was fixed with 827219a1c35b4aca6dbab5855a36e9277b2ec8f4 (on master) and on 7.6 already, too. So this can be closed, I’m just going to add some more tests.

#17 Updated by Andreas Wolf about 3 years ago

  • Status changed from Under Review to Closed

#18 Updated by Sascha Rademacher over 2 years ago

Can you please reopen that?

I think this Bugfix is absolutly wrong and fatal. The Date and the Datetime Field in DB has no Timezone, so why convert to UTC and then convert back? This makes Dates in DB not readable and destroys already running systems. I want to take care of convertion by myself and I never had Problems with that.

What about this bug, which i can confirm (Viktor Livakivskyi):

There is a special case with DB fields of type 'date'.

Imagine a case, when a DateTime object is created in "Europe/Berlin" like this

$dateObject = new \DateTime('2016-07-13');

This will correspond to 2016-07-13 00:00:00.000000 in timezone "Europe/Berlin".
So, in case ExtBase will convert it to UTC during persistence, the date will correspond to 2016-07-12 22:00:00.000000 in UTC timezone. And saved to DB will result in "2016-07-12" stored.

Then on retrieval from DB the DateTime object will be created and Timezone shift added, which will correspond to 2016-07-12 02:00:00.000000 in "Europe/Berlin".
One day is lost then.

So, for case with native DB 'date' field the timezone shift should be totally ignored.

And what about the Suggedtion from "This Mächler":

Suggestion:
Make it possible to choose how TYPO3 should treat dateTime values:

1. TYPO3 assumes that dateTime values from the DB are in local time and need no conversion from/to the FE or BE. This should be the default setting to avoid breaking a running system.

2. TYPO3 assumes that dateTime values are stored in UTC, conversion takes place from/to the FE or BE.

This will not solve the problem with the already implemented workarounds. Maybe there is no solution for that.

My already running systems crash with this Bugfix. If you think this change is correct, then please resprect the suggestion from "This Mächler".

#19 Updated by Markus Klein over 2 years ago

  • Parent task deleted (#77562)

@Sascha: Would you mind opening a new ticket for this. Please set this one as related ticket and describe the issue. Feel free to add Andreas Wolf and me a watchers.

#20 Updated by Markus Klein over 2 years ago

  • Related to Bug #79613: Saving wrong Date into DB if Field is of type DATE added

#21 Updated by Markus Klein over 2 years ago

Ah just saw that you already did so: #79613

Also available in: Atom PDF