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

Fix tz parse failure from out of bounds error #8

Merged
merged 1 commit into from Jul 14, 2019

Conversation

@joshleeb
Copy link
Contributor

commented Jul 6, 2019

This PR fixes the index out of bounds error that occurs when parsing
some specific timezones.

Tested by running the code to reproduce in #7 as well as compiling exa
with zoneinfo-compiled containing this patch to confirm that exa --long doesn't panic.

Resolves #7
Relevant downstream ogham/exa#517

Fix tz parse failure from out of bounds error
This PR fixes the `index out of bounds` error that occurs when parsing
some specific timezones.

Tested by running the code to reproduce in #7 as well as compiling exa
with zoneinfo-compiled containing this patch to confirm that `exa
--long` doesn't panic.
@joshleeb

This comment has been minimized.

Copy link
Contributor Author

commented Jul 6, 2019

I don't have much knowledge on zoneinfo files but this seems to fix the issue, and the downstream issue with exa, at least at the surface level.

@ogham ogham merged commit d43e28e into rust-datetime:master Jul 14, 2019

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@ogham

This comment has been minimized.

Copy link
Collaborator

commented Jul 14, 2019

Oh my god, I spent like an hour debugging this problem today because I didn't see this PR!

I've merged this because I prefer having those two expressions as variables now that I look at it. The good news is we ended up with pretty much the same solution:

transition_type:  flags_to_transition_type(tz.standard_flags.get(i).unwrap_or(&0) != &0,
                                           tz.gmt_flags.get(i).unwrap_or(&0) != &0),

I wanted to look up how the official zoneinfo code handles this case, and the code confirms that if those values are missing, they should be assumed to be 0: https://github.com/eggert/tz/blob/f9bd527016f806611e58a99d759d00c64c46a938/tzfile.h#L70-L80

Anyway, thanks for this. I'm going to publish a release today to fix the exa issue.

@ogham

This comment has been minimized.

Copy link
Collaborator

commented Jul 14, 2019

v0.4.8 is out

@ogham ogham referenced this pull request Jul 14, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.