-
Notifications
You must be signed in to change notification settings - Fork 168
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
Improve TzIf format detection #237
Improve TzIf format detection #237
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi, thank you for taking your time to contribute.
The problem I tried to describe in #214 is that the detection is too loose. Any simple text file that happens to start with TZif
is detected as application/tzif
(see the commit I added on this PR, it shows the problem.)
https://tools.ietf.org/id/draft-murchison-tzdist-tzif-00.html#rfc.section.3 describes in detail the format of the tzif file. For example, fifth byte, version
, must be 0x00, 0x32 or 0x33, charcnt
bytes must not be 0, etc.
These are the checks that we can do for tzif files to make the detection more accurate.
Hello @gabriel-vasile, Thanks for creating a test case. I did not fully understand the problem, and I think I do now. I created some additional checks:
|
Additional header checks: - Header length - Version - typecnt MUST not be zero
Sorry for overwriting twice, my signature was not working properly because of a new system install. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi, code looks better but there are still some issues to fix. I left some suggestions.
You can commit how many times you want, there is not problem.
Co-authored-by: Gabriel Vasile <[email protected]> Co-authored-by: Gabriel Vasile <[email protected]>
Code is easier to read and faster to run. Co-authored-by: Gabriel Vasile <[email protected]>
Test case was used to show the problem when detecting a TzIf file. The test is no longer needed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for your contribution!
Added the four-octet ASCII sequence to identify TzIf files:
0x54 0x5A 0x69 0x66
Source