From kde-core-devel Fri Mar 14 17:04:47 2014 From: "David Jarvie" Date: Fri, 14 Mar 2014 17:04:47 +0000 To: kde-core-devel Subject: Re: Review Request 116784: Fix incorrect use of KDateTime.toTime_t in kio_http Message-Id: <20140314170447.2591.93011 () probe ! kde ! org> X-MARC-Message: https://marc.info/?l=kde-core-devel&m=139481676503575 MIME-Version: 1 Content-Type: multipart/mixed; boundary="--===============2261915031650456710==" --===============2261915031650456710== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116784/#review52940 ----------------------------------------------------------- kioslave/http/http.cpp Need to convert to UTC first, since it will produce the wrong value if it's in some random time zone: dt.toUtc().dateTime() - David Jarvie On March 13, 2014, 12:49 p.m., Dawit Alemayehu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/116784/ > ----------------------------------------------------------- > > (Updated March 13, 2014, 12:49 p.m.) > > > Review request for kdelibs, Andreas Hartmetz and David Faure. > > > Repository: kdelibs > > > Description > ------- > > The attached patch does the following: > > - It corrects a mistake in assumption that KDateTime.toTime_t() will return -1 for invalidate dates. It does not. The result is an overflow which is interpreted in kio_http as a timestamp in the distant future which obviously is wrong. See https://bugs.kde.org/show_bug.cgi?id=331774 for example. This assumption also affects the timestamp variables used for cache management. > > - It converts cache management timestamp variables to 64 bits so they can accomodates dates beyond Feb 7, 2106. > > > Diffs > ----- > > kioslave/http/http.h dd85622 > kioslave/http/http.cpp e4f1eba > > Diff: https://git.reviewboard.kde.org/r/116784/diff/ > > > Testing > ------- > > > Thanks, > > Dawit Alemayehu > > --===============2261915031650456710== Content-Type: text/html; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit

This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116784/

kioslave/http/http.cpp (Diff revision 1)
		304	return (dt.isValid() ? (dt.dateTime().toMSecsSinceEpoch()/1000) : -1);

Need to convert to UTC first, since it will produce the wrong value if it's in some random time zone:
dt.toUtc().dateTime()

- David Jarvie

On March 13th, 2014, 12:49 p.m. UTC, Dawit Alemayehu wrote:

Review request for kdelibs, Andreas Hartmetz and David Faure.

By Dawit Alemayehu.

Updated March 13, 2014, 12:49 p.m.

Repository: kdelibs

Description

The attached patch does the following:

- It corrects a mistake in assumption that KDateTime.toTime_t() will return -1 for invalidate dates. It does not. The result is an overflow which is interpreted in kio_http as a timestamp in the distant future which obviously is wrong. See https://bugs.kde.org/show_bug.cgi?id=331774 for example. This assumption also affects the timestamp variables used for cache management.

- It converts cache management timestamp variables to 64 bits so they can accomodates dates beyond Feb 7, 2106.

Diffs

kioslave/http/http.h (dd85622)
kioslave/http/http.cpp (e4f1eba)

--===============2261915031650456710==--