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

populate_token_attributes() ValueError for float expires_at values #745

Open
ggiill opened this issue Dec 10, 2020 · 8 comments
Open

populate_token_attributes() ValueError for float expires_at values #745

ggiill opened this issue Dec 10, 2020 · 8 comments
Labels
Bug Contributor Friendly OAuth2-Client This impact the client part of OAuth2.
Milestone

Comments

@ggiill
Copy link

ggiill commented Dec 10, 2020

self._expires_at = int(response.get('expires_at'))

Some tokens have an expires_at value as a float (example: 1612807300.1613762) which causes this line to throw a ValueError: invalid literal for int() with base 10. This could be solved using int(float(response.get('expires_at'))) (or rounding to not truncate).

@wibek
Copy link

wibek commented Feb 23, 2021

Just above that self._expires_at is made a float:

self._expires_at = time.time() + int(self.expires_in)

@JonathanHuot
Copy link
Member

The usage of expires_at is not defined in RFC, so it seems confusion is in the code as well. We can assume it is an int similarly to expires_in.

Any PR to fix its usage and add unit test to cover this edge-case is welcome.

@JonathanHuot JonathanHuot added Bug Contributor Friendly OAuth2-Client This impact the client part of OAuth2. labels Feb 25, 2021
@ajayd-san
Copy link

hey, can i have this issue? I'm just starting with open source, so this will be a good start ig.

@ggiill
Copy link
Author

ggiill commented Apr 27, 2021

hey, can i have this issue? I'm just starting with open source, so this will be a good start ig.

@default-303 go for it!

@ajayd-san
Copy link

@ggiill thanks, I did the following changes

- self._expires_at = int(response.get('expires_at')) 
+ self._expires_at = round(float(response.get('expires_at')))

and passes all the tests is when i run pytest

But what kinda tests you want me to add ? like a simple typecheck test something like this -

import unittest
import fix  ## a sample script i made up for this demo
import time

class TestFix(unittest.TestCase) : 

    def test_float(self) : 
        string_time = str(time.time())
        self.assertIsInstance(fix.get_time("123345.1222"), int)

    def test_int(self) : 
        string_time = str(round(time.time()))
        self.assertIsInstance(fix.get_time("12333"), int)

or a full on mock test ?

@ajayd-san
Copy link

@ggiill hey, can i get a review ?
lmao its been a month even i forgot abt it.

@ggiill
Copy link
Author

ggiill commented May 30, 2021

@ggiill hey, can i get a review ?
lmao its been a month even i forgot abt it.

@default-303 I'm not a maintainer - pinging @JonathanHuot. You probably want to put in a PR for review as well.

@JonathanHuot
Copy link
Member

@default-303 : would you mind submitting a PR that we can easily review & discuss ? Thanks in advance!

sindrig added a commit to sindrig/oauthlib that referenced this issue Aug 22, 2022
@JonathanHuot JonathanHuot added this to the 3.3.0 milestone Sep 6, 2022
auvipy pushed a commit that referenced this issue Jun 13, 2023
JonathanHuot pushed a commit that referenced this issue Aug 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Contributor Friendly OAuth2-Client This impact the client part of OAuth2.
Projects
None yet
Development

No branches or pull requests

4 participants