Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Backups list incorrect time #721

Closed
wants to merge 1 commit into from
Closed

Backups list incorrect time #721

wants to merge 1 commit into from

Conversation

pdewouters
Copy link
Contributor

A user did some debugging on BWP and this was his result.

I have now looked at your code and determined the problem is caused by using 'date' in conjunction with current_time. The timezone offset is getting applied twice, once in each function.

When given a timestamp, 'date' expects it to be the seconds from when the unix clock began, GMT. It then provides local offset time.
http://php.net/manual/en/function.date.php

Current_time does the same thing. It expects seconds from epoch, GMT and it applies the offset.
http://codex.wordpress.org/Function_Reference/current_time

In PHP 5.1+ date_default_timezone started having an impact. It is settable on the system (mine is default) and it is settable in code, which wordpress does on the settings page. When I call date_default_timezone_get() from within wordpress it correctly reports Denver / MST / -7.
http://php.net/manual/en/function.date-default-timezone-set.php
http://vip.wordpress.com/documentation/vip-development-tips-tricks/use-current_time-not-date_default_timezone_set/

In several classes you use:
date('Y-m-d H-i-s',current_time('timestamp'))
This results in a 2x Offset.

Instead you should use one of the following options:

date('Y-m-d H-i-s', current_time('timestamp', 1))

current_time('Y-m-d H-i-s', 1)

gmdate('Y-m-d H-i-s', current_time('timestamp'))

@willmot
Copy link
Contributor

willmot commented Jan 29, 2015

Good catch

@pdewouters
Copy link
Contributor

Yeah, so we should use date( 'Y-m-d H-i-s', time() ); as time() is equivalent to current_time( 'timestamp', true )

@willmot
Copy link
Contributor

willmot commented Jan 30, 2015

Sounds like a good candidate for a 3.1.3 release if 3.2 isn't ready by next week.

@pdewouters
Copy link
Contributor

The places where we were doing this is only to set the filename, not the schedule.
My mind's not very clear at the moment but could the issue be here?
https://github.com/humanmade/backupwordpress/blob/master/classes/class-schedule.php#L581

@pdewouters
Copy link
Contributor

So, just tested this - the code seems fine to me on master. If I set the WordPress timezone to UTC-5 and run a backup, it displays the correct time in the finished backups table and the backup file name is also correct with the right time.
On this branch however it is not.

@pdewouters
Copy link
Contributor

Timezone set to UTC-5

screen shot 2015-02-02 at 15 17 37
screen shot 2015-02-02 at 15 17 10

screen shot 2015-02-02 at 15 25 38

@pdewouters pdewouters deleted the issue-721 branch March 30, 2015 10:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants