:Gijs (he/him) Assignee
	Description • 9 years ago

+++ This bug was initially created as a clone of Bug #1153900 +++

At least for me on Win8, the expiry time is turned into a date which we pass to Services.cookies.add, which turns it into a timestamp, which we then multiply by 1000 before turning it back into a date, leading to it being really high (year 40000-odd).

I'm not sure if this is a regression or not, but from the code I suspect it's been like this for a long time, and nobody noticed...

	Marco Bonardo [:mak]
	Comment 1 • 9 years ago

I guess the usual timestamp vs PRTime...

Btw, I have ctypes working for the test, I can set a cookie with an expire time, so this should be testable too.

	:Gijs (he/him) Assignee
	Comment 2 • 9 years ago

Tested, upcoming patch fixes things on my local Win8 IE11 machine. I'll report back with a WinXP test
in a bit, but it seems likely that this is the correct fix everywhere - if the format had changed on the IE side, the new Date() call would never have worked in the first place, I assume...

	:Gijs (he/him) Assignee
	Comment 3 • 9 years ago

/r/7057 - Bug 1154472 - fix expiry dates in IE cookie imports, r?mak

Pull down this commit:

hg pull -r bdb3880ff357cd57640928aca89979ac5a257ee9 https://reviewboard-hg.mozilla.org/gecko/

	:Gijs (he/him) Assignee
	Comment 4 • 9 years ago

Actually... while we're here, I think this gets timezones wrong still? Maybe? Let me doublecheck...

	:Gijs (he/him) Assignee
	Comment 5 • 9 years ago

(In reply to :Gijs Kruitbosch from comment #4)
> Actually... while we're here, I think this gets timezones wrong still?
> Maybe? Let me doublecheck...

Yup, both debugging and https://msdn.microsoft.com/en-us/library/windows/desktop/ms724280%28v=vs.85%29.aspx indicate that this will be returning UTC, and the new Date() constructor will use local time. I'll update the patch.

	:Gijs (he/him) Assignee
	Comment 6 • 9 years ago

Comment on attachment 8592728 [details]
MozReview Request: bz://1154472/Gijs

/r/7057 - Bug 1154472 - fix expiry dates in IE cookie imports, r?mak

Pull down this commit:

hg pull -r e6dc44a067161a17c517b87edda490bb84b1011f https://reviewboard-hg.mozilla.org/gecko/

	Marco Bonardo [:mak]
	Comment 7 • 9 years ago

Comment on attachment 8592728 [details]
MozReview Request: bz://1154472/Gijs

https://reviewboard.mozilla.org/r/7055/#review5869

::: browser/components/migration/IEProfileMigrator.js
(Diff revision 2)
> -  fileTimeToDate: function CH_fileTimeToDate(aTimeHi, aTimeLo) {
> +  fileTimeToSecondsSinceEpoch: function CH_fileTimeToSecondsSinceEpoch(aTimeHi, aTimeLo) {

nit: I'm fine if you wish to shorthand to

fileTimeToSecondsSinceEpoch(aTimeHi, aTimeLo) {

	Marco Bonardo [:mak]
	Comment 8 • 9 years ago

https://hg.mozilla.org/integration/fx-team/rev/f18502092330

has an automated test in bug 1154294

	Pulsebot
	Comment 9 • 9 years ago

https://hg.mozilla.org/integration/fx-team/rev/f18502092330

	Ryan VanderMeulen [:RyanVM]
	Comment 10 • 9 years ago

https://hg.mozilla.org/mozilla-central/rev/f18502092330