Skip to content

Commit f848096

Browse files
committed
fix rate limit headers - were ints/floats need to be strings
Running under gunicorn rest requests were crashing. Not all of the values for the rate limit headers were strings. Some were numbers. This caused the header generation for wsgi to fail. Now the values are all strings.
1 parent 36ddab5 commit f848096

File tree

4 files changed

+27
-24
lines changed

4 files changed

+27
-24
lines changed

roundup/cgi/actions.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1264,7 +1264,7 @@ def handle(self):
12641264
if reject:
12651265
# User exceeded limits: find out how long to wait
12661266
status=gcra.status(rlkey, limit)
1267-
raise Reject(_("Logins occurring too fast. Please wait: %d seconds.")%status['Retry-After'])
1267+
raise Reject(_("Logins occurring too fast. Please wait: %s seconds.")%status['Retry-After'])
12681268

12691269
self.verifyLogin(self.client.user, password)
12701270
except exceptions.LoginError as err:

roundup/rate_limit.py

Lines changed: 14 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -79,39 +79,42 @@ def status(self, key, limit):
7979

8080
ret = {}
8181
tat = self.get_tat(key)
82-
8382
# static defined headers according to limit
84-
ret['X-RateLimit-Limit'] = limit.count
85-
ret['X-RateLimit-Limit-Period'] = int(limit.period.total_seconds())
83+
# all values are strings as that is required when used as headers
84+
ret['X-RateLimit-Limit'] = str(limit.count)
85+
ret['X-RateLimit-Limit-Period'] = str(
86+
int(
87+
limit.period.total_seconds())
88+
)
8689

8790
# status of current limit as of now
8891
now = datetime.utcnow()
8992

90-
ret['X-RateLimit-Remaining'] = min(int(
91-
(limit.period - (tat - now)).total_seconds() \
92-
/limit.inverse),ret['X-RateLimit-Limit'])
93+
current_count = int((limit.period - (tat - now)).total_seconds()\
94+
/limit.inverse)
95+
ret['X-RateLimit-Remaining'] = str(min(current_count,limit.count))
9396

9497
# tat_in_epochsec = (tat - datetime(1970, 1, 1)).total_seconds()
9598
seconds_to_tat = (tat - now).total_seconds()
96-
ret['X-RateLimit-Reset'] = max(seconds_to_tat, 0)
99+
ret['X-RateLimit-Reset'] = str(max(seconds_to_tat, 0))
97100
ret['X-RateLimit-Reset-date'] = "%s"%tat
98-
ret['Now'] = (now - datetime(1970,1,1)).total_seconds()
101+
ret['Now'] = str((now - datetime(1970,1,1)).total_seconds())
99102
ret['Now-date'] = "%s"%now
100103

101104
if self.update(key, limit, testonly=True):
102105
# A new request would be rejected if it was processes.
103106
# The user has to wait until an item is dequeued.
104107
# One item is dequeued every limit.inverse seconds.
105-
ret['Retry-After'] = limit.inverse
108+
ret['Retry-After'] = str(int(limit.inverse))
106109
ret['Retry-After-Timestamp'] = "%s"%(now + timedelta(seconds=limit.inverse))
107110
else:
108111
# if we are not rejected, the user can post another
109112
# attempt immediately.
110113
# Do we even need this header if not rejected?
111114
# RFC implies this is used with a 503 (or presumably
112115
# 429 which may postdate the rfc). So if no error, no header?
113-
# ret['Retry-After'] = 0
114-
# ret['Retry-After-Timestamp'] = ret['Now-date']
116+
# ret['Retry-After'] = '0'
117+
# ret['Retry-After-Timestamp'] = str(ret['Now-date'])
115118
pass
116119

117120
return ret

roundup/rest.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1778,7 +1778,7 @@ def dispatch(self, method, uri, input):
17781778
# User exceeded limits: tell humans how long to wait
17791779
# Headers above will do the right thing for api
17801780
# aware clients.
1781-
msg=_("Api rate limits exceeded. Please wait: %d seconds.")%limitStatus['Retry-After']
1781+
msg=_("Api rate limits exceeded. Please wait: %s seconds.")%limitStatus['Retry-After']
17821782
output = self.error_obj(429, msg, source="ApiRateLimiter")
17831783
else:
17841784
for header,value in limitStatus.items():

test/rest_common.py

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1001,7 +1001,7 @@ def testRestRateLimit(self):
10011001
# remaining count is correct
10021002
self.assertEqual(
10031003
self.server.client.additional_headers["X-RateLimit-Remaining"],
1004-
self.db.config['WEB_API_CALLS_PER_INTERVAL'] -1 - i
1004+
str(self.db.config['WEB_API_CALLS_PER_INTERVAL'] -1 - i)
10051005
)
10061006

10071007
# trip limit
@@ -1014,20 +1014,20 @@ def testRestRateLimit(self):
10141014

10151015
self.assertEqual(
10161016
self.server.client.additional_headers["X-RateLimit-Limit"],
1017-
self.db.config['WEB_API_CALLS_PER_INTERVAL'])
1017+
str(self.db.config['WEB_API_CALLS_PER_INTERVAL']))
10181018
self.assertEqual(
10191019
self.server.client.additional_headers["X-RateLimit-Limit-Period"],
1020-
self.db.config['WEB_API_INTERVAL_IN_SEC'])
1020+
str(self.db.config['WEB_API_INTERVAL_IN_SEC']))
10211021
self.assertEqual(
10221022
self.server.client.additional_headers["X-RateLimit-Remaining"],
1023-
0)
1023+
'0')
10241024
# value will be almost 60. Allow 1-2 seconds for all 20 rounds.
10251025
self.assertAlmostEqual(
1026-
self.server.client.additional_headers["X-RateLimit-Reset"],
1026+
float(self.server.client.additional_headers["X-RateLimit-Reset"]),
10271027
59, delta=1)
10281028
self.assertEqual(
10291029
str(self.server.client.additional_headers["Retry-After"]),
1030-
"3.0") # check as string
1030+
"3") # check as string
10311031

10321032
print("Reset:", self.server.client.additional_headers["X-RateLimit-Reset"])
10331033
print("Now realtime pre-sleep:", datetime.utcnow())
@@ -1049,18 +1049,18 @@ def testRestRateLimit(self):
10491049

10501050
self.assertEqual(
10511051
self.server.client.additional_headers["X-RateLimit-Limit"],
1052-
self.db.config['WEB_API_CALLS_PER_INTERVAL'])
1052+
str(self.db.config['WEB_API_CALLS_PER_INTERVAL']))
10531053
self.assertEqual(
10541054
self.server.client.additional_headers["X-RateLimit-Limit-Period"],
1055-
self.db.config['WEB_API_INTERVAL_IN_SEC'])
1055+
str(self.db.config['WEB_API_INTERVAL_IN_SEC']))
10561056
self.assertEqual(
10571057
self.server.client.additional_headers["X-RateLimit-Remaining"],
1058-
0)
1058+
'0')
10591059
self.assertFalse("Retry-After" in
10601060
self.server.client.additional_headers)
10611061
# we still need to wait a minute for everything to clear
10621062
self.assertAlmostEqual(
1063-
self.server.client.additional_headers["X-RateLimit-Reset"],
1063+
float(self.server.client.additional_headers["X-RateLimit-Reset"]),
10641064
59, delta=1)
10651065

10661066
# and make sure we need to wait another three seconds
@@ -1072,7 +1072,7 @@ def testRestRateLimit(self):
10721072
self.assertEqual(self.server.client.response_code, 429)
10731073
self.assertEqual(
10741074
str(self.server.client.additional_headers["Retry-After"]),
1075-
"3.0") # check as string
1075+
"3") # check as string
10761076

10771077
json_dict = json.loads(b2s(results))
10781078
self.assertEqual(json_dict['error']['msg'],

0 commit comments

Comments
 (0)