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

Windows timestamp in converter panel #114

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Windows timestamp in converter panel #114

wants to merge 4 commits into from

Conversation

sealmove
Copy link
Member

No description provided.

@sealmove sealmove mentioned this pull request Apr 21, 2020
@GreyCat
Copy link
Member

GreyCat commented Apr 22, 2020

Thanks for this contribution!

I'm somewhat concerned about the name choice: "Windows timestamp". Windows-related products are known to use several different models of tracking time, and the one that you've provided is the one generally known as FILETIME, i.e. 64-bit integer measuring time since 1601-01-01 in 100ns ticks.

Let us use more precise naming to avoid the confusion?

@sealmove
Copy link
Member Author

Is filets an appropriate name to display? I tried FILETIME but it looks ugly.

@generalmimon
Copy link
Member

generalmimon commented Jun 27, 2020

@sealmove:

Is filets an appropriate name to display?

I'd rather pick filetime, because we have to assume that not everyone will immediately understand what the abbreviation means, so they will presumably search it on Google. And you can imagine that you can't find anything relevant by searching for "filets", but when you enter "filetime", the first link is what we want.

And the two extra characters don't really matter, when we consider that we reduce a lot of confusion in our users.

Comment on lines +87 to +88
var u64le = Converter.numConv(data, 8, false, false);
this.filetime = u64le ? dateFormat(new Date(Converter.fileToUnixTime(parseInt(u64le)) * 1000), "yyyy-mm-dd HH:MM:ss") : "";
Copy link
Member

@generalmimon generalmimon Jul 4, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sealmove Unfonutately, as all numbers in JavaScript are doubles with just 53 bits of mantissa, a 64-bit integer will lose precision during the conversion to it (using the parseInt method). It means that not all integers larger than 253 - 1 (available in the Number.MAX_SAFE_INTEGER constant) can't be represented accurately in the native JavaScript number type: e.g. Number.MAX_SAFE_INTEGER + 1 == Number.MAX_SAFE_INTEGER + 2, see MDN.

But it doesn't have to happen here, because the Converter.numConv method works internally with a bigInt, though it returns a string (don't know why). Anyway, I would suggest to convert the string to the bigInt type, and do the arithmetics you need to do in the fileToUnixTime method using its methods instead of the native arithmetic operators.

And I just checked that Date object has more than sufficient range for not losing precision here, it's even far larger than FILETIME has (maximum year in FILETIME is ~30828, maximum year of Date is 275760), so there shouldn't be a problem, if you choose a safe method of initializing it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there shouldn't be a problem, if you choose a safe method of initializing it.

@sealmove Actually, you can safely ignore that "safe method". I've just tested on RunKit the maximum year 30828 of FILETIME, and the number that represents that Date (number of milliseconds that have elapsed since midnight on January 1, 1970, UTC) is still ~10 times less than the Number.MAX_SAFE_INTEGER, so it will be fine to just stuff that Unix timestamp * 1000 number into the new Date() constructor, after you do the arithmetic with bigInt.

@sem-geologist
Copy link

I wish this feature would be included in webide, hate updating the version as I need to add that every time I update (I am RE formats which always have some MS FILETIME).
I would call it in gui just like that 'msfiletime' ; I think adding ms before filetime would not hurt as that then is even more unambiguous about the meaning.
I think it looks ok (see in the screenshot):
kaitai_ms_filetime_screenshot

Another issue I think with proposed implementation is that it adds unnecessary function for something what can be hardcoded. The relation between unixtime and MS FILETIME is fixed and can't be changed (unless Someone would find the way altering time flow on MS and unix machines, but leave that to SciFi).
I personally after updating kaitai ide on my PC add this code to script below the unixts script:

var u64le = Converter.numConv(data, 8, false, false);
this.msfiletime = u64le ? dateFormat(new Date(parseInt(u64le)/10000 - 11644473600000), "yyyy-mm-dd HH:MM:ss") : "";

no new functions defined for (re-)calculating constant relation. Works OK.

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.

4 participants