Skip to content

Commit 1a87077

Browse files
committed
Better error handling in Credentials.from_json.
I dropped an over-broad `except` by only catching errors that `datetime.datetime.strptime` would throw -- forgetting that it might error differently if one of its arguments were an unexpected type. Fix and add a test.
1 parent 8868a23 commit 1a87077

File tree

2 files changed

+9
-2
lines changed

2 files changed

+9
-2
lines changed

oauth2client/client.py

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -225,7 +225,8 @@ def _to_json(self, strip):
225225
for member in strip:
226226
if member in d:
227227
del d[member]
228-
if 'token_expiry' in d and isinstance(d['token_expiry'], datetime.datetime):
228+
if (d.get('token_expiry') and
229+
isinstance(d['token_expiry'], datetime.datetime)):
229230
d['token_expiry'] = d['token_expiry'].strftime(EXPIRY_FORMAT)
230231
# Add in information we will need later to reconsistitue this instance.
231232
d['_class'] = t.__name__
@@ -589,7 +590,7 @@ def from_json(cls, s):
589590
An instance of a Credentials subclass.
590591
"""
591592
data = json.loads(s)
592-
if ('token_expiry' in data and
593+
if (data.get('token_expiry') and
593594
not isinstance(data['token_expiry'], datetime.datetime)):
594595
try:
595596
data['token_expiry'] = datetime.datetime.strptime(

tests/test_oauth2client.py

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -587,6 +587,12 @@ def test_to_from_json(self):
587587

588588
self.assertEqual(instance.__dict__, self.credentials.__dict__)
589589

590+
def test_from_json_token_expiry(self):
591+
data = json.loads(self.credentials.to_json())
592+
data['token_expiry'] = None
593+
instance = OAuth2Credentials.from_json(json.dumps(data))
594+
self.assertTrue(isinstance(instance, OAuth2Credentials))
595+
590596
def test_no_unicode_in_request_params(self):
591597
access_token = u'foo'
592598
client_id = u'some_client_id'

0 commit comments

Comments
 (0)