[PATCH] kdecore: Fix a bug in KDateTime utc offset string parsing.
The sign of the UTC offset was ignored, and an offset of -0500 (New York) would be treated at +0500 (Pakistan). This commit also adds a unit test for UTC offset parsing and comparasion. --- Hi When mucking around in the frameworks branch of kdelibs I found a bug in the KDateTime string parsing, which appears to be present in master as well as every branch from 4.0 to 4.10. I have, however, only run the updated unittest using the frameworks branch and Qt5, so someone else should probably test on 4.9, 4.10 and/or master before committing it. Best Regards Jon Severinsson kdecore/date/kdatetime.cpp |2 +- kdecore/tests/kdatetimetest.cpp |7 +++ 2 filer ändrade, 8 tillägg(+), 1 borttagning(-) diff --git a/kdecore/date/kdatetime.cpp b/kdecore/date/kdatetime.cpp index d4f63ff..df1fc3d 100644 --- a/kdecore/date/kdatetime.cpp +++ b/kdecore/date/kdatetime.cpp @@ -2962,7 +2962,7 @@ bool getUTCOffset(const QString string, int offset, bool colon, int result) tzmin += tzhour * 60; if (result != NO_NUMBERresult != tzmin) return false; -result = tzmin; +result = sign * tzmin; return true; } diff --git a/kdecore/tests/kdatetimetest.cpp b/kdecore/tests/kdatetimetest.cpp index e79e9f2..812abc5 100644 --- a/kdecore/tests/kdatetimetest.cpp +++ b/kdecore/tests/kdatetimetest.cpp @@ -3807,6 +3807,13 @@ void KDateTimeTest::strings_format() QVERIFY(!dt.isValid());// too early QVERIFY(dt.outOfRange()); +dtutc = KDateTime::fromString(QLatin1String(2000-01-01T00:00:00.000+), QLatin1String(%Y-%m-%dT%H:%M%:S%:s%z)); +QVERIFY(dtutc.isValid()); +dt = KDateTime::fromString(QLatin1String(2000-01-01T05:00:00.000+0500), QLatin1String(%Y-%m-%dT%H:%M%:S%:s%z)); +QVERIFY(dt.isValid()); +QVERIFY(dtutc == dt); +dt = KDateTime::fromString(QLatin1String(1999-12-31T20:30:00.000-0330), QLatin1String(%Y-%m-%dT%H:%M%:S%:s%z)); +QVERIFY(dtutc == dt); // Restore the original local time zone if (originalZone.isEmpty()) -- 1.7.10.4
Re: [PATCH] kdecore: Fix a bug in KDateTime utc offset string parsing.
On Sunday, October 07, 2012 22:23:56 Jon Severinsson wrote: The sign of the UTC offset was ignored, and an offset of -0500 (New York) would be treated at +0500 (Pakistan). This commit also adds a unit test for UTC offset parsing and comparasion. --- Hi When mucking around in the frameworks branch of kdelibs I found a bug in the KDateTime string parsing, which appears to be present in master as well as every branch from 4.0 to 4.10. I have, however, only run the updated unittest using the frameworks branch and Qt5, so someone else should probably test on 4.9, 4.10 and/or master before committing it. I've adapted the patch slightly to the 4.9 testsuite and I have verified that the new test does fail without your patch, and passes with it. I can commit the test and fix to 4.9 and 4.10, but I would like it if someone with experience in the date/time code could review first. The fix makes sense to me, I just don't know if there are other affected areas or unintended breakage that would be experienced. I've CC'ed David Jarvie but anyone else with experience can chime in too. :P Regards, - Michael Pyne signature.asc Description: This is a digitally signed message part.
Re: [PATCH] kdecore: Fix a bug in KDateTime utc offset string parsing.
On Sunday 07 October 2012 21:23:56 Jon Severinsson wrote: The sign of the UTC offset was ignored, and an offset of -0500 (New York) would be treated at +0500 (Pakistan). This commit also adds a unit test for UTC offset parsing and comparasion. --- Hi When mucking around in the frameworks branch of kdelibs I found a bug in the KDateTime string parsing, which appears to be present in master as well as every branch from 4.0 to 4.10. I have, however, only run the updated unittest using the frameworks branch and Qt5, so someone else should probably test on 4.9, 4.10 and/or master before committing it. Best Regards Jon Severinsson kdecore/date/kdatetime.cpp |2 +- kdecore/tests/kdatetimetest.cpp |7 +++ 2 filer ändrade, 8 tillägg(+), 1 borttagning(-) diff --git a/kdecore/date/kdatetime.cpp b/kdecore/date/kdatetime.cpp index d4f63ff..df1fc3d 100644 --- a/kdecore/date/kdatetime.cpp +++ b/kdecore/date/kdatetime.cpp @@ -2962,7 +2962,7 @@ bool getUTCOffset(const QString string, int offset, bool colon, int result) tzmin += tzhour * 60; if (result != NO_NUMBERresult != tzmin) return false; -result = tzmin; +result = sign * tzmin; return true; } diff --git a/kdecore/tests/kdatetimetest.cpp b/kdecore/tests/kdatetimetest.cpp index e79e9f2..812abc5 100644 --- a/kdecore/tests/kdatetimetest.cpp +++ b/kdecore/tests/kdatetimetest.cpp @@ -3807,6 +3807,13 @@ void KDateTimeTest::strings_format() QVERIFY(!dt.isValid());// too early QVERIFY(dt.outOfRange()); +dtutc = KDateTime::fromString(QLatin1String(2000-01-01T00:00:00.000+), QLatin1String(%Y-%m-%dT%H:%M%:S%:s%z)); +QVERIFY(dtutc.isValid()); +dt = KDateTime::fromString(QLatin1String(2000-01-01T05:00:00.000+0500), QLatin1String(%Y-%m-%dT%H:%M%:S%:s%z)); +QVERIFY(dt.isValid()); +QVERIFY(dtutc == dt); +dt = KDateTime::fromString(QLatin1String(1999-12-31T20:30:00.000-0330), QLatin1String(%Y-%m-%dT%H:%M%:S%:s%z)); +QVERIFY(dtutc == dt); // Restore the original local time zone if (originalZone.isEmpty()) The patches look good to commit. However, can you please move the new tests in kdatetimetest.cpp to start at line 3740, before the existing group of tests for UTC offsets. This will keep related tests grouped together. -- David Jarvie. KDE developer. KAlarm author -- http://www.astrojar.org.uk/kalarm
