ntptime: Year 2036 fix. #830
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jonfoster
force-pushed
the
master
branch
3 times, most recently
from
6d56d52 to
16572e3
Compare
|
Commit message fixed to comply with the coding standards. (I hope). |
Fix NTP client - it would report the wrong time after 7 Feb 2036. Signed-off-by: Jon Foster <[email protected]>
With the old code, if you make a request with HTTP basic auth (a username/password) and did not specify a headers dict, then the cleartext username and password would be added to the default headers to be used for every subsequent HTTP request. That's probably not a good idea. This fixes it so the headers dict passed in, whether default or not, is never changed.
HTTPS requests made with this module are hideously insecure.
The HTTP User-Agent string is very useful if malfunctioning IoT devices start putting excessive load on a server. It allows the server operator to ban just the malfunctioning devices, not all users. It's also a SHOULD in the HTTP specification. Add it, enabled by default, but with options to override or disable it.
The HTTP response headers are case-insensitive. But the old implementation put them in a dictionary keyed by case-sensitive header name. The only way to correctly get a header would have been to iterate through the dictionary looking for it, which is slow and a lot of code and I doubt anyone bothered, I expect that existing code probably just did a case-sensitive match. That's fragile, server changes could break it. To fix that, lowercase all the header names before putting them in the dictionary. That way, clients can just access the dictionary with the lowercase header name and it will do the right thing. This is backward compatible with correctly-written existing code. But it's not compatible with existing code that was buggy anyway and did a case-sensitive match.
This is intended to make it easy to read the Last-Modified header, but it would work just as well for any of the other date-formatted HTTP response headers.
There's code there that doesn't work properly with timezones. The fix is left for the future, but at least I want to mark it as dodgy to make it more likely I remember to go back and fix it.
|
Sorry, I didn't intend for all those WIP commits to go into this PR. Will try to sort that out. |
Merged
|
Moved to #837 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fix a bug in the NTP client, where it would report the wrong time after 7 Feb 2036.
The NTP timestamp has a 32-bit count of seconds, which will wrap back to zero on 7 Feb 2036 at 06:28:16.
It's now March 2024. So we know that timestamps less than 1st Jan 2024 are impossible. So if the timestamp appears to be earlier than 1st Jan 2024, that probably means that the NTP time wrapped at 2^32 seconds. (Or someone set the wrong time on their NTP server, but we can't really do anything about that). So in that case, we need to add in those extra 2^32 seconds, to get the correct timestamp.
This means that this code will work until the year 2160.
(More precisely, this code will not work after 7th Feb 2160 at 06:28:15. Fixing that is someone else's problem, some time in the far future, by adjusting the NTP_DELTA and MIN_NTP_TIMESTAMP values. By just changing those two numbers, you can choose any 136-year span of time that will be supported).