<html><head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
</head>
<body>
<p>*with the underflow from atoi error handled. <br>
<br>
</p>
# HG changeset patch<br>
# User <a class="moz-txt-link-abbreviated" href="mailto:jordanc.carter@outlook.com">jordanc.carter@outlook.com</a><br>
# Date 1673146061 0<br>
# Sun Jan 08 02:47:41 2023 +0000<br>
# Branch dynamic-rate-limiting<br>
# Node ID 72cabfd1397057a598af8925a335b80b7eba2da1<br>
# Parent 07b0bee87f32be91a33210bc06973e07c4c1dac9<br>
Variables support for limit_req_zone's rate<br>
<br>
diff -r 07b0bee87f32 -r 72cabfd13970
src/http/modules/ngx_http_limit_req_module.c<br>
--- a/src/http/modules/ngx_http_limit_req_module.c Wed Dec 21
14:53:27 2022 +0300<br>
+++ b/src/http/modules/ngx_http_limit_req_module.c Sun Jan 08
02:47:41 2023 +0000<br>
@@ -26,6 +26,7 @@<br>
/* integer value, 1 corresponds to 0.001 r/s */<br>
ngx_uint_t excess;<br>
ngx_uint_t count;<br>
+ ngx_uint_t rate;<br>
u_char data[1];<br>
} ngx_http_limit_req_node_t;<br>
<br>
@@ -41,6 +42,7 @@<br>
ngx_http_limit_req_shctx_t *sh;<br>
ngx_slab_pool_t *shpool;<br>
/* integer value, 1 corresponds to 0.001 r/s */<br>
+ ngx_http_complex_value_t cvr;<br>
ngx_uint_t rate;<br>
ngx_http_complex_value_t key;<br>
ngx_http_limit_req_node_t *node;<br>
@@ -66,9 +68,9 @@<br>
<br>
static void ngx_http_limit_req_delay(ngx_http_request_t *r);<br>
static ngx_int_t
ngx_http_limit_req_lookup(ngx_http_limit_req_limit_t *limit,<br>
- ngx_uint_t hash, ngx_str_t *key, ngx_uint_t *ep, ngx_uint_t
account);<br>
+ ngx_uint_t hash, ngx_str_t *key, ngx_uint_t *ep, ngx_uint_t
account, ngx_uint_t rate);<br>
static ngx_msec_t
ngx_http_limit_req_account(ngx_http_limit_req_limit_t *limits,<br>
- ngx_uint_t n, ngx_uint_t *ep, ngx_http_limit_req_limit_t
**limit);<br>
+ ngx_uint_t n, ngx_uint_t *ep, ngx_http_limit_req_limit_t
**limit, ngx_uint_t rate);<br>
static void ngx_http_limit_req_unlock(ngx_http_limit_req_limit_t
*limits,<br>
ngx_uint_t n);<br>
static void ngx_http_limit_req_expire(ngx_http_limit_req_ctx_t
*ctx,<br>
@@ -195,10 +197,12 @@<br>
ngx_http_limit_req_handler(ngx_http_request_t *r)<br>
{<br>
uint32_t hash;<br>
- ngx_str_t key;<br>
+ ngx_str_t key, s;<br>
ngx_int_t rc;<br>
ngx_uint_t n, excess;<br>
+ ngx_uint_t scale, rate;<br>
ngx_msec_t delay;<br>
+ u_char *p;<br>
ngx_http_limit_req_ctx_t *ctx;<br>
ngx_http_limit_req_conf_t *lrcf;<br>
ngx_http_limit_req_limit_t *limit, *limits;<br>
@@ -211,6 +215,7 @@<br>
limits = lrcf->limits.elts;<br>
<br>
excess = 0;<br>
+ rate = 0;<br>
<br>
rc = NGX_DECLINED;<br>
<br>
@@ -243,10 +248,44 @@<br>
<br>
hash = ngx_crc32_short(key.data, key.len);<br>
<br>
+ if (ctx->rate == 0) {<br>
+<br>
+ scale = 1;<br>
+<br>
+ if (ngx_http_complex_value(r, &ctx->cvr, &s)
!= NGX_OK) {<br>
+ ngx_http_limit_req_unlock(limits, n);<br>
+ return NGX_HTTP_INTERNAL_SERVER_ERROR;<br>
+ }<br>
+<br>
+ if (s.len < 9) {<br>
+ continue;<br>
+ }<br>
+<br>
+ rate = (ngx_uint_t) ngx_atoi(s.data + 5, s.len - 8);<br>
+<br>
+ if (rate == 0 || rate == (ngx_uint_t) NGX_ERROR) {<br>
+ continue;<br>
+ }<br>
+<br>
+ p = s.data + s.len - 3;<br>
+<br>
+ if (ngx_strncmp(p, "r/m", 3) == 0) {<br>
+ scale = 60;<br>
+<br>
+ } else if (ngx_strncmp(p, "r/s", 3) != 0) {<br>
+ continue;<br>
+ }<br>
+<br>
+ rate = rate * 1000 / scale;<br>
+<br>
+ } else {<br>
+ rate = ctx->rate;<br>
+ }<br>
+<br>
ngx_shmtx_lock(&ctx->shpool->mutex);<br>
<br>
rc = ngx_http_limit_req_lookup(limit, hash, &key,
&excess,<br>
- (n == lrcf->limits.nelts
- 1));<br>
+ (n == lrcf->limits.nelts
- 1), rate);<br>
<br>
ngx_shmtx_unlock(&ctx->shpool->mutex);<br>
<br>
@@ -291,7 +330,7 @@<br>
excess = 0;<br>
}<br>
<br>
- delay = ngx_http_limit_req_account(limits, n, &excess,
&limit);<br>
+ delay = ngx_http_limit_req_account(limits, n, &excess,
&limit, rate);<br>
<br>
if (!delay) {<br>
r->main->limit_req_status =
NGX_HTTP_LIMIT_REQ_PASSED;<br>
@@ -403,7 +442,7 @@<br>
<br>
static ngx_int_t<br>
ngx_http_limit_req_lookup(ngx_http_limit_req_limit_t *limit,
ngx_uint_t hash,<br>
- ngx_str_t *key, ngx_uint_t *ep, ngx_uint_t account)<br>
+ ngx_str_t *key, ngx_uint_t *ep, ngx_uint_t account, ngx_uint_t
rate)<br>
{<br>
size_t size;<br>
ngx_int_t rc, excess;<br>
@@ -451,7 +490,9 @@<br>
ms = 0;<br>
}<br>
<br>
- excess = lr->excess - ctx->rate * ms / 1000 +
1000;<br>
+ excess = lr->excess - rate * ms / 1000 + 1000;<br>
+<br>
+ lr->rate = rate;<br>
<br>
if (excess < 0) {<br>
excess = 0;<br>
@@ -510,6 +551,7 @@<br>
<br>
lr->len = (u_short) key->len;<br>
lr->excess = 0;<br>
+ lr->rate = rate;<br>
<br>
ngx_memcpy(lr->data, key->data, key->len);<br>
<br>
@@ -534,7 +576,7 @@<br>
<br>
static ngx_msec_t<br>
ngx_http_limit_req_account(ngx_http_limit_req_limit_t *limits,
ngx_uint_t n,<br>
- ngx_uint_t *ep, ngx_http_limit_req_limit_t **limit)<br>
+ ngx_uint_t *ep, ngx_http_limit_req_limit_t **limit, ngx_uint_t
rate)<br>
{<br>
ngx_int_t excess;<br>
ngx_msec_t now, delay, max_delay;<br>
@@ -548,8 +590,7 @@<br>
max_delay = 0;<br>
<br>
} else {<br>
- ctx = (*limit)->shm_zone->data;<br>
- max_delay = (excess - (*limit)->delay) * 1000 /
ctx->rate;<br>
+ max_delay = (excess - (*limit)->delay) * 1000 / rate;<br>
}<br>
<br>
while (n--) {<br>
@@ -572,7 +613,7 @@<br>
ms = 0;<br>
}<br>
<br>
- excess = lr->excess - ctx->rate * ms / 1000 + 1000;<br>
+ excess = lr->excess - lr->rate * ms / 1000 + 1000;<br>
<br>
if (excess < 0) {<br>
excess = 0;<br>
@@ -593,7 +634,7 @@<br>
continue;<br>
}<br>
<br>
- delay = (excess - limits[n].delay) * 1000 / ctx->rate;<br>
+ delay = (excess - limits[n].delay) * 1000 / lr->rate;<br>
<br>
if (delay > max_delay) {<br>
max_delay = delay;<br>
@@ -676,7 +717,7 @@<br>
return;<br>
}<br>
<br>
- excess = lr->excess - ctx->rate * ms / 1000;<br>
+ excess = lr->excess - lr->rate * ms / 1000;<br>
<br>
if (excess > 0) {<br>
return;<br>
@@ -860,7 +901,7 @@<br>
}<br>
<br>
size = 0;<br>
- rate = 1;<br>
+ rate = 0;<br>
scale = 1;<br>
name.len = 0;<br>
<br>
@@ -902,6 +943,20 @@<br>
<br>
if (ngx_strncmp(value[i].data, "rate=", 5) == 0) {<br>
<br>
+ ngx_memzero(&ccv,
sizeof(ngx_http_compile_complex_value_t));<br>
+<br>
+ ccv.cf = cf;<br>
+ ccv.value = &value[i];<br>
+ ccv.complex_value = &ctx->cvr;<br>
+<br>
+ if (ngx_http_compile_complex_value(&ccv) != NGX_OK)
{<br>
+ return NGX_CONF_ERROR;<br>
+ }<br>
+<br>
+ if (ctx->cvr.lengths != NULL) {<br>
+ continue;<br>
+ }<br>
+<br>
len = value[i].len;<br>
p = value[i].data + len - 3;<br>
<br>
<br>
<div class="moz-cite-prefix">On 07/01/2023 06:04, J Carter wrote:<br>
</div>
<blockquote type="cite" cite="mid:7451de9a-8f0a-5f96-09ac-2370289e9cde@outlook.com">Hello,
<br>
<br>
Please find below my revised patch with style and code bug fixes
included. The only change in behavior of note is that invalid rate
values are now simply ignored in the same fashion an empty key is,
and the use of the parsing of complex value being disabled if a
non variable is used as the rate.
<br>
<br>
A brief overview of the issue and how I've resolved it to match
the behavior of the current best solution for multiple rates.
<br>
<br>
> "The fundamental problem with dynamic rates in limit_req,
which is
<br>
> a leaky bucket limiter, is that the rate is a property which
<br>
> applies to multiple requests as accumulated in the bucket
(and
<br>
> this is basically why it is configured in the limit_req_zone,
and
<br>
> not in limit_req), while the dynamically calculated rate is a
<br>
> property of the last request.
<br>
>
<br>
> This can easily result in counter-intuitive behaviour.
<br>
> For example, consider 5 requests with 1 second interval,
assuming
<br>
> burst 2, and rate being 1r/m or 100r/s depending on some
request
<br>
> properties:
<br>
>
<br>
> - 1st request, rate 1r/m, request is accepted
<br>
> - 2st request, rate 1r/m, request is accepted
<br>
> - 3rd request, rate 1r/m, request is rejected, since there is
no
<br>
> room in the bucket
<br>
> - 4th request, rate 100r/s, request is accepted
<br>
> - 5th request, rate 1r/m, request is accepted (unexpectedly)
<br>
>
<br>
> Note that the 5th request is accepted, while it is mostly
<br>
> equivalent to the 3rd request, and one could expect it to be
<br>
> rejected. But it happens to to be accepted because the 4th
<br>
> request "cleared" the bucket."
<br>
<br>
No additional logic was required to fix this. Simply appending the
$rate variable to the $key is sufficient for a user to avoid
unwanted excess value changes with extreme rate jumps (as seen
above).
<br>
<br>
Instead of: limit_req_zone $binary_remote_addr zone=one:10m
rate=$rate;
<br>
<br>
this: limit_req_zone $binary_remote_addr$rate
zone=one:10m rate=$rate;
<br>
<br>
This ensures that a change in the rate variable's value starts new
accounting of excess (as a new state is created to reflect this
new key) - this follows the behavior previously given in this
chain for a static set of rates. As an additional benefit, the
syntax is more compact and readable within the nginx configuration
at the cost of slight overhead in memory.
<br>
<br>
<br>
(the solution previously given for multiple defined/static rates)
<br>
<br>
> ...
<br>
> map $traffic_tier $limit_free {
<br>
> free $binary_remote_addr;
<br>
> }
<br>
>
<br>
> map $traffic_tier $limit_basic {
<br>
> basic $binary_remote_addr;
<br>
> }
<br>
>
<br>
> map $traffic_tier $limit_premium {
<br>
> premium $binary_remote_addr;
<br>
> }
<br>
>
<br>
> limit_req_zone $limit_free zone=free:10m rate=1r/m;
<br>
> limit_req_zone $limit_basic zone=basic:10m rate=2r/m;
<br>
> limit_req_zone $limit_premium zone=premium:10m rate=1r/s;
<br>
>
<br>
> limit_req zone=free (burst=x nodelay);
<br>
> limit_req zone=basic (burst=x nodelay);
<br>
> limit_req zone=premium (burst=x nodelay);
<br>
<br>
to
<br>
<br>
...
<br>
<br>
map $traffic_tier $rate {
<br>
free 1r/m;
<br>
basic 2r/m;
<br>
premium 1r/s;
<br>
}
<br>
<br>
limit_req_zone $binary_remote_addr$rate zone=one:10m rate=$rate;
<br>
limit_req zone=one burst=x nodelay;
<br>
<br>
(With the appended $rate after $key being optional - if your rates
don't flip flop to extremes omitting it will save memory and allow
more states)
<br>
<br>
The is merely a side benefit - the main purpose of the patch is
being able to obtain a rate limit in the from a string (of some
description) from a request, or any variable that is populated in
time for the limit_req module to use directly as a rate limit
value. The approach detailed above works for that too.
<br>
<br>
<br>
debian@debian:~/projects/nginx-merc/nginx-tests$ prove limit_req*
<br>
limit_req2.t ......... ok
<br>
limit_req_delay.t .... ok
<br>
limit_req_dry_run.t .. ok
<br>
limit_req.t .......... ok
<br>
All tests successful.
<br>
Files=4, Tests=40, 5 wallclock secs ( 0.05 usr 0.01 sys + 0.48
cusr 0.12 csys = 0.66 CPU)
<br>
Result: PASS
<br>
<br>
# HG changeset patch
<br>
# User <a class="moz-txt-link-abbreviated" href="mailto:jordanc.carter@outlook.com">jordanc.carter@outlook.com</a>
<br>
# Date 1673064770 0
<br>
# Sat Jan 07 04:12:50 2023 +0000
<br>
# Branch dynamic-rate-limiting
<br>
# Node ID f80741fb734e0a4f83f2f96436ff300c4f3125aa
<br>
# Parent 07b0bee87f32be91a33210bc06973e07c4c1dac9
<br>
Variables support for limit_req_zone's rate
<br>
<br>
diff -r 07b0bee87f32 -r f80741fb734e
src/http/modules/ngx_http_limit_req_module.c
<br>
--- a/src/http/modules/ngx_http_limit_req_module.c Wed Dec 21
14:53:27 2022 +0300
<br>
+++ b/src/http/modules/ngx_http_limit_req_module.c Sat Jan 07
04:12:50 2023 +0000
<br>
@@ -26,6 +26,7 @@
<br>
/* integer value, 1 corresponds to 0.001 r/s */
<br>
ngx_uint_t excess;
<br>
ngx_uint_t count;
<br>
+ ngx_uint_t rate;
<br>
u_char data[1];
<br>
} ngx_http_limit_req_node_t;
<br>
<br>
@@ -41,6 +42,7 @@
<br>
ngx_http_limit_req_shctx_t *sh;
<br>
ngx_slab_pool_t *shpool;
<br>
/* integer value, 1 corresponds to 0.001 r/s */
<br>
+ ngx_http_complex_value_t cvr;
<br>
ngx_uint_t rate;
<br>
ngx_http_complex_value_t key;
<br>
ngx_http_limit_req_node_t *node;
<br>
@@ -66,9 +68,9 @@
<br>
<br>
static void ngx_http_limit_req_delay(ngx_http_request_t *r);
<br>
static ngx_int_t
ngx_http_limit_req_lookup(ngx_http_limit_req_limit_t *limit,
<br>
- ngx_uint_t hash, ngx_str_t *key, ngx_uint_t *ep, ngx_uint_t
account);
<br>
+ ngx_uint_t hash, ngx_str_t *key, ngx_uint_t *ep, ngx_uint_t
account, ngx_uint_t rate);
<br>
static ngx_msec_t
ngx_http_limit_req_account(ngx_http_limit_req_limit_t *limits,
<br>
- ngx_uint_t n, ngx_uint_t *ep, ngx_http_limit_req_limit_t
**limit);
<br>
+ ngx_uint_t n, ngx_uint_t *ep, ngx_http_limit_req_limit_t
**limit, ngx_uint_t rate);
<br>
static void ngx_http_limit_req_unlock(ngx_http_limit_req_limit_t
*limits,
<br>
ngx_uint_t n);
<br>
static void ngx_http_limit_req_expire(ngx_http_limit_req_ctx_t
*ctx,
<br>
@@ -195,10 +197,12 @@
<br>
ngx_http_limit_req_handler(ngx_http_request_t *r)
<br>
{
<br>
uint32_t hash;
<br>
- ngx_str_t key;
<br>
+ ngx_str_t key, s;
<br>
ngx_int_t rc;
<br>
ngx_uint_t n, excess;
<br>
+ ngx_uint_t scale, rate;
<br>
ngx_msec_t delay;
<br>
+ u_char *p;
<br>
ngx_http_limit_req_ctx_t *ctx;
<br>
ngx_http_limit_req_conf_t *lrcf;
<br>
ngx_http_limit_req_limit_t *limit, *limits;
<br>
@@ -211,6 +215,7 @@
<br>
limits = lrcf->limits.elts;
<br>
<br>
excess = 0;
<br>
+ rate = 0;
<br>
<br>
rc = NGX_DECLINED;
<br>
<br>
@@ -243,10 +248,44 @@
<br>
<br>
hash = ngx_crc32_short(key.data, key.len);
<br>
<br>
+ if (ctx->rate == 0) {
<br>
+
<br>
+ scale = 1;
<br>
+
<br>
+ if (ngx_http_complex_value(r, &ctx->cvr,
&s) != NGX_OK) {
<br>
+ ngx_http_limit_req_unlock(limits, n);
<br>
+ return NGX_HTTP_INTERNAL_SERVER_ERROR;
<br>
+ }
<br>
+
<br>
+ if (s.len < 9) {
<br>
+ continue;
<br>
+ }
<br>
+
<br>
+ rate = (ngx_uint_t) ngx_atoi(s.data + 5, s.len - 8);
<br>
+
<br>
+ if (rate == 0) {
<br>
+ continue;
<br>
+ }
<br>
+
<br>
+ p = s.data + s.len - 3;
<br>
+
<br>
+ if (ngx_strncmp(p, "r/m", 3) == 0) {
<br>
+ scale = 60;
<br>
+
<br>
+ } else if (ngx_strncmp(p, "r/s", 3) != 0) {
<br>
+ continue;
<br>
+ }
<br>
+
<br>
+ rate = rate * 1000 / scale;
<br>
+
<br>
+ } else {
<br>
+ rate = ctx->rate;
<br>
+ }
<br>
+
<br>
ngx_shmtx_lock(&ctx->shpool->mutex);
<br>
<br>
rc = ngx_http_limit_req_lookup(limit, hash, &key,
&excess,
<br>
- (n ==
lrcf->limits.nelts - 1));
<br>
+ (n ==
lrcf->limits.nelts - 1), rate);
<br>
<br>
ngx_shmtx_unlock(&ctx->shpool->mutex);
<br>
<br>
@@ -291,7 +330,7 @@
<br>
excess = 0;
<br>
}
<br>
<br>
- delay = ngx_http_limit_req_account(limits, n, &excess,
&limit);
<br>
+ delay = ngx_http_limit_req_account(limits, n, &excess,
&limit, rate);
<br>
<br>
if (!delay) {
<br>
r->main->limit_req_status =
NGX_HTTP_LIMIT_REQ_PASSED;
<br>
@@ -403,7 +442,7 @@
<br>
<br>
static ngx_int_t
<br>
ngx_http_limit_req_lookup(ngx_http_limit_req_limit_t *limit,
ngx_uint_t hash,
<br>
- ngx_str_t *key, ngx_uint_t *ep, ngx_uint_t account)
<br>
+ ngx_str_t *key, ngx_uint_t *ep, ngx_uint_t account,
ngx_uint_t rate)
<br>
{
<br>
size_t size;
<br>
ngx_int_t rc, excess;
<br>
@@ -451,7 +490,9 @@
<br>
ms = 0;
<br>
}
<br>
<br>
- excess = lr->excess - ctx->rate * ms / 1000 +
1000;
<br>
+ excess = lr->excess - rate * ms / 1000 + 1000;
<br>
+
<br>
+ lr->rate = rate;
<br>
<br>
if (excess < 0) {
<br>
excess = 0;
<br>
@@ -510,6 +551,7 @@
<br>
<br>
lr->len = (u_short) key->len;
<br>
lr->excess = 0;
<br>
+ lr->rate = rate;
<br>
<br>
ngx_memcpy(lr->data, key->data, key->len);
<br>
<br>
@@ -534,7 +576,7 @@
<br>
<br>
static ngx_msec_t
<br>
ngx_http_limit_req_account(ngx_http_limit_req_limit_t *limits,
ngx_uint_t n,
<br>
- ngx_uint_t *ep, ngx_http_limit_req_limit_t **limit)
<br>
+ ngx_uint_t *ep, ngx_http_limit_req_limit_t **limit,
ngx_uint_t rate)
<br>
{
<br>
ngx_int_t excess;
<br>
ngx_msec_t now, delay, max_delay;
<br>
@@ -548,8 +590,7 @@
<br>
max_delay = 0;
<br>
<br>
} else {
<br>
- ctx = (*limit)->shm_zone->data;
<br>
- max_delay = (excess - (*limit)->delay) * 1000 /
ctx->rate;
<br>
+ max_delay = (excess - (*limit)->delay) * 1000 / rate;
<br>
}
<br>
<br>
while (n--) {
<br>
@@ -572,7 +613,7 @@
<br>
ms = 0;
<br>
}
<br>
<br>
- excess = lr->excess - ctx->rate * ms / 1000 + 1000;
<br>
+ excess = lr->excess - lr->rate * ms / 1000 + 1000;
<br>
<br>
if (excess < 0) {
<br>
excess = 0;
<br>
@@ -593,7 +634,7 @@
<br>
continue;
<br>
}
<br>
<br>
- delay = (excess - limits[n].delay) * 1000 / ctx->rate;
<br>
+ delay = (excess - limits[n].delay) * 1000 / lr->rate;
<br>
<br>
if (delay > max_delay) {
<br>
max_delay = delay;
<br>
@@ -676,7 +717,7 @@
<br>
return;
<br>
}
<br>
<br>
- excess = lr->excess - ctx->rate * ms / 1000;
<br>
+ excess = lr->excess - lr->rate * ms / 1000;
<br>
<br>
if (excess > 0) {
<br>
return;
<br>
@@ -860,7 +901,7 @@
<br>
}
<br>
<br>
size = 0;
<br>
- rate = 1;
<br>
+ rate = 0;
<br>
scale = 1;
<br>
name.len = 0;
<br>
<br>
@@ -902,6 +943,20 @@
<br>
<br>
if (ngx_strncmp(value[i].data, "rate=", 5) == 0) {
<br>
<br>
+ ngx_memzero(&ccv,
sizeof(ngx_http_compile_complex_value_t));
<br>
+
<br>
+ ccv.cf = cf;
<br>
+ ccv.value = &value[i];
<br>
+ ccv.complex_value = &ctx->cvr;
<br>
+
<br>
+ if (ngx_http_compile_complex_value(&ccv) !=
NGX_OK) {
<br>
+ return NGX_CONF_ERROR;
<br>
+ }
<br>
+
<br>
+ if (ctx->cvr.lengths != NULL) {
<br>
+ continue;
<br>
+ }
<br>
+
<br>
len = value[i].len;
<br>
<pre> p = value[i].data + len - 3;
</pre>
<br>
<br>
<br>
<br>
</blockquote>
</body>
</html>