Skip to content

Fix timezone calculation for non-integer timezones #236

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

Merged
merged 2 commits into from
Jun 22, 2020

Conversation

mackuba
Copy link
Contributor

@mackuba mackuba commented Nov 29, 2018

There are two places in the code which apply a timezone difference to a parsed date: when parsing command line arguments (Configuration._set_date) and when processing a hit (Parser.parse). They do that in a rather hacky way: the timezone is parsed as a number and then the difference is set to be number / 100 hours.

This kind of works... except it assumes that timezones can only be at full integer intervals, and that's not true. There are several non-integer timezones, including in such important countries as India (+05:30) or Iran (+03:30). In that case, "0530" is interpreted as 5.3 hours ahead of UTC instead of 5.5...

For example, with such log line:

1.2.3.4 - - [29/Nov/2018:15:30:00 +0530] "GET / HTTP/1.1" 200 6355 "-" "Mozilla/5.0 (Windows NT 6.1; WOW64; rv:43.0) Gecko/20100101 Firefox/43.0" 0.014

the 15:30 Indian Standard Time should be read as 10:00 UTC (11:00 CET). Instead, it's 11:12:

screenshot 2018-11-29 at 17 18 41

This PR makes a relatively small fix to the timezone calculating logic in that the parsed "number" (not really an actual number) e.g. 530 is divided into hours and minutes separately using integer division and modulo and passed to datetime.timedelta as separate parameters. With that change, the timestamp is parsed as 10:00 UTC (11:00 CET) correctly:

screenshot 2018-11-29 at 17 52 48

@mattab mattab added this to the Current sprint milestone Mar 26, 2019
@sgiehl
Copy link
Member

sgiehl commented Oct 21, 2019

The changes look good for me. But as I'm not working with python regularly maybe some else could have a look

@tsteur tsteur changed the base branch from master to 3.x-dev January 13, 2020 22:45
@diosmosis diosmosis merged commit 12b98e8 into matomo-org:3.x-dev Jun 22, 2020
@innocraft-automation innocraft-automation removed this from the Current sprint milestone Jan 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants