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

Guard against obscure bug in test setup #3308

Closed
wants to merge 1 commit into from
Closed

Conversation

quis
Copy link
Member

@quis quis commented Feb 18, 2020

If you try to freeze time to before 2011 you get the following obscure exception from somewhere within itsdangerous:

timestamp = base64_encode(int_to_bytes(self.get_timestamp()))
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

num = -1

def int_to_bytes(num):
>       assert num >= 0
E       AssertionError

Something in there is baselining timestamp to 0 at the start of 2011. Which means that trying to freeze time to before then results in a negative timestamp.

This is a very non-obvious exception to see, so hopefully raising a specific exception instead will save others some time.

If you try to freeze time to before 2011 you get the following obscure
exception from somewhere within itsdangerous:

```
timestamp = base64_encode(int_to_bytes(self.get_timestamp()))
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

num = -1

def int_to_bytes(num):
>       assert num >= 0
E       AssertionError
```

Something in there is baselining `timestamp` to `0` at the start of
2011. Which means that trying to freeze time to before then results in a
negative timestamp.

This is a very non-obvious exception to see, so hopefully raising a
specific exception instead will save others some time.
@quis quis force-pushed the guard-freezetime-bug branch from 5317e5a to 238142f Compare February 18, 2020 12:03
@klssmith
Copy link
Contributor

It seems like the timestamp thing was a bug in the version of itsdangerous that we are using that has now been fixed (pallets/itsdangerous#46). So we could just try and bump itsdangerous instead - we shouldn't need to pin it to 0.24 anymore because version 1.1.0 has undone the breaking changes that version 1.0.0 introduced. (Haven't tried unpinning it to see if it all works though...)

@quis quis mentioned this pull request Feb 19, 2020
@quis quis closed this in #3314 Apr 1, 2020
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.

2 participants