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

FormatInternalDate outputs wrong formatted dates with timezones having negative offsets #1753

Conversation

elkoiko
Copy link
Contributor

@elkoiko elkoiko commented May 15, 2024

First of all

First of all, thank you very much for your work on MailKit. I'm very pleased to have the opportunity to contribute to it !
Now, let's see what I found 😃.

Issue

The FormatInternalDate method has a bug when handling time zones with negative offsets where the minutes component was non-zero. This resulted in incorrectly formatted outputs. Specifically, the method produced an invalid format with an extra negative sign for the minutes part. Examples of the incorrect outputs included:

Expected: 14-May-2023 12:30:45 -0430
But was: 14-May-2023 12:30:45 -04-30

Expected: 15-May-2023 12:30:45 -0330
But was: 15-May-2023 12:30:45 -03-30

These incorrect outputs are associated with time zones such as:

  • Venezuela Standard Time (VET), which is UTC-04:30.
  • Newfoundland Standard Time (NST), which is UTC-03:30.

Fix

To address this issue, I formatted the time zone offset beforehand to ensure the correct inclusion of the sign and the proper formatting of hours and minutes in an invariant culture way. Along the process, I also created unit tests that confirm that the fix does not introduce regressions.

@jstedfast
Copy link
Owner

I think a simpler solution is to just use Math.Abs() on the offset minutes:

return string.Format (CultureInfo.InvariantCulture, "{0:D2}-{1}-{2:D4} {3:D2}:{4:D2}:{5:D2} {6:+00;-00}{7:00}",
				date.Day, Months[date.Month - 1], date.Year, date.Hour, date.Minute, date.Second,
				date.Offset.Hours, Math.Abs (date.Offset.Minutes));

This would require fewer string allocations.

code formatting
@jstedfast jstedfast merged commit 8414973 into jstedfast:master May 16, 2024
4 of 5 checks passed
@jstedfast
Copy link
Owner

Awesome, thanks for the fix!

@elkoiko elkoiko deleted the format-internal-date-issue-negative-timezone branch May 16, 2024 14:07
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.

2 participants