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

Parse more timezones #868

Merged
merged 4 commits into from
Jan 3, 2023
Merged

Conversation

endgame
Copy link
Collaborator

@endgame endgame commented Jan 3, 2023

Change the format string for RFC822 to use %Z for any timezone instead of the literal GMT. The use of GMT in that format string goes as far back as I can trace (at least 8 years), so I'm surprised to see errors beginning now. Regardless, it's valid RFC 822 so let's support it.

Fixes #866

It seems that AWS (maybe just S3?) can sometimes return RFC5322-format
dates with an `obs-zone` other than "GMT", so widen our parser to
accept them.

A `%Z` in the format string causes `formatTime` to write the time zone
as `UTC`, which is not a `zone` under RFC 822 nor an `obs-zone` in
RFC 5322. We therefore split the `TimeFormat` class to distinguish
between parsing and printing formats, so that we continue to emit the
`obs-zone` of `GMT`.
@endgame
Copy link
Collaborator Author

endgame commented Jan 3, 2023

@hasufell please test this PR and let me know if it fixes your issue.

@hasufell
Copy link

hasufell commented Jan 3, 2023

@hasufell please test this PR and let me know if it fixes your issue.

I'm afraid I can't test this easily, because the libraries that I use don't build with amazonka HEAD and I don't have the time to invest in updating all that.

Given that you have a test case using that exact date string, I guess it's probably fine.

@endgame endgame merged commit af17036 into brendanhay:main Jan 3, 2023
@endgame endgame deleted the parse-more-timezones branch January 3, 2023 21:24
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.

Serialize error: Failure parsing Time from value
2 participants