limit_rate_after support variables

Ruslan Ermilov ru at nginx.com
Thu Dec 6 14:45:22 UTC 2018


On Wed, Nov 21, 2018 at 03:50:30PM +0100, Miroslav Novy wrote:
> Hi Ruslan,
>   there is my fix to remove warning "using uninitialized variable".
> 
> I have moved the reading value of limit_rate variable from function
> ngx_http_update_location_config to function ngx_http_write_filter
> because value is initialized later.
> 
> Best regards
> Miroslav Nový
> 
> # HG changeset patch
> # User Miroslav Nový <miranovy at gmail.com>
> # Date 1542811768 0
> #      Wed Nov 21 14:49:28 2018 +0000
> # Node ID e2139cd62c7263dc182b11a847a6eaf2d9560a0d
> # Parent  82eed7650622fd780dcd4a86661de7b80b44199c
> Variables support in limit_rate and limit_rate_after fix
> 
> Reading value of limit_rate variable moved to function
> ngx_http_write_filter because in function
> ngx_http_update_location_config is not inicialized yet.
> 
> diff -r 82eed7650622 -r e2139cd62c72 src/http/ngx_http_core_module.c
> --- a/src/http/ngx_http_core_module.c Wed Nov 21 10:40:01 2018 +0000
> +++ b/src/http/ngx_http_core_module.c Wed Nov 21 14:49:28 2018 +0000
> @@ -1212,8 +1212,6 @@
>  void
>  ngx_http_update_location_config(ngx_http_request_t *r)
>  {
> -    size_t                     limit_rate;
> -    ngx_str_t                  val;
>      ngx_http_core_loc_conf_t  *clcf;
> 
>      clcf = ngx_http_get_module_loc_conf(r, ngx_http_core_module);
> @@ -1283,20 +1281,6 @@
>          r->connection->tcp_nopush = NGX_TCP_NOPUSH_DISABLED;
>      }
> 
> -    if (r->limit_rate == 0
> -        && clcf->limit_rate
> -        && ngx_http_complex_value_size(r, clcf->limit_rate, &val, &limit_rate)
> -           == NGX_OK)
> -    {
> -        if (limit_rate != (size_t) NGX_ERROR) {
> -            r->limit_rate = limit_rate;
> -
> -        } else if (val.len) {
> -            ngx_log_error(NGX_LOG_ERR, r->connection->log, 0,
> -                          "invalid \"limit_rate\" value \"%V\"", &val);
> -        }
> -    }
> -
>      if (clcf->handler) {
>          r->content_handler = clcf->handler;
>      }
> diff -r 82eed7650622 -r e2139cd62c72 src/http/ngx_http_write_filter_module.c
> --- a/src/http/ngx_http_write_filter_module.c Wed Nov 21 10:40:01 2018 +0000
> +++ b/src/http/ngx_http_write_filter_module.c Wed Nov 21 14:49:28 2018 +0000
> @@ -48,7 +48,7 @@
>  ngx_http_write_filter(ngx_http_request_t *r, ngx_chain_t *in)
>  {
>      off_t                      size, sent, nsent, limit;
> -    size_t                     limit_rate_after;
> +    ssize_t                    limit_rate, limit_rate_after;
>      ngx_str_t                  val;
>      ngx_uint_t                 last, flush, sync;
>      ngx_msec_t                 delay;
> @@ -220,6 +220,25 @@
>          return NGX_ERROR;
>      }
> 
> +    if (r->limit_rate == 0
> +         && clcf->limit_rate
> +         && ngx_http_complex_value_size(r, clcf->limit_rate, &val,
> +                                        &limit_rate)
> +         == NGX_OK)
> +     {
> +         if (limit_rate_after != NGX_ERROR) {

You're checking the wrong variable here.

> +             r->limit_rate = limit_rate;
> +
> +         } else if (val.len) {
> +             ngx_log_error(NGX_LOG_ERR, c->log, 0,
> +                           "invalid \"limit_rate\" value \"%V\"",
> +                           &val);
> +         }
> +     }
> +
> +     ngx_log_debug1(NGX_LOG_DEBUG_HTTP, c->log, 0, "http limit rate \"%z\"",
> +                           r->limit_rate);
> +

Unfortunately, moving it here from the write filter the way it's done
breaks unbuffered proxying combined with limit_rate set.

>      if (r->limit_rate) {
>          if (r->limit_rate_after == 0
>              && clcf->limit_rate_after
> @@ -227,7 +246,7 @@
>                                             &limit_rate_after)
>                 == NGX_OK)
>          {
> -            if (limit_rate_after != (size_t) NGX_ERROR) {
> +            if (limit_rate_after != NGX_ERROR) {
>                  r->limit_rate_after = limit_rate_after;
> 
>              } else if (val.len) {

Here's my take.  I didn't change size_t vs. ssize_t as I believe
it's okay the way I did it.

# HG changeset patch
# User Ruslan Ermilov <ru at nginx.com>
# Date 1544099131 -10800
#      Thu Dec 06 15:25:31 2018 +0300
# Node ID 1a8c0a959f636b5eea0a4f42711af08591c23a99
# Parent  2117637f64e981e0e14c3a4b0509252fefd8a78a
Added ngx_http_set_complex_value_size_slot().

If a complex value is expected to be size_t, and the compiled value
is constant, the constant size_t value is remembered at compile time.

The value is accessed through ngx_http_complex_value_size() which
either returns the remembered constant or evaluates the expression
and parses it as size_t.

diff --git a/src/http/ngx_http_script.c b/src/http/ngx_http_script.c
--- a/src/http/ngx_http_script.c
+++ b/src/http/ngx_http_script.c
@@ -105,6 +105,25 @@ ngx_http_complex_value(ngx_http_request_
 
 
 ngx_int_t
+ngx_http_complex_value_size(ngx_http_request_t *r,
+    ngx_http_complex_value_t *val, ngx_str_t *value, size_t *size)
+{
+    if (val->lengths == NULL) {
+        *size = val->u.size;
+        return NGX_OK;
+    }
+
+    if (ngx_http_complex_value(r, val, value) != NGX_OK) {
+        return NGX_ERROR;
+    }
+
+    *size = ngx_parse_size(value);
+
+    return NGX_OK;
+}
+
+
+ngx_int_t
 ngx_http_compile_complex_value(ngx_http_compile_complex_value_t *ccv)
 {
     ngx_str_t                  *v;
@@ -244,6 +263,36 @@ ngx_http_set_complex_value_slot(ngx_conf
 }
 
 
+char *
+ngx_http_set_complex_value_size_slot(ngx_conf_t *cf, ngx_command_t *cmd,
+    void *conf)
+{
+    char  *p = conf;
+
+    char                      *rv;
+    ngx_http_complex_value_t  *cv;
+
+    rv = ngx_http_set_complex_value_slot(cf, cmd, conf);
+
+    if (rv != NGX_CONF_OK) {
+        return rv;
+    }
+
+    cv = *(ngx_http_complex_value_t **) (p + cmd->offset);
+
+    if (cv->lengths) {
+        return NGX_CONF_OK;
+    }
+
+    cv->u.size = ngx_parse_size(&cv->value);
+    if (cv->u.size == (size_t) NGX_ERROR) {
+        return "invalid value";
+    }
+
+    return NGX_CONF_OK;
+}
+
+
 ngx_int_t
 ngx_http_test_predicates(ngx_http_request_t *r, ngx_array_t *predicates)
 {
diff --git a/src/http/ngx_http_script.h b/src/http/ngx_http_script.h
--- a/src/http/ngx_http_script.h
+++ b/src/http/ngx_http_script.h
@@ -68,6 +68,10 @@ typedef struct {
     ngx_uint_t                 *flushes;
     void                       *lengths;
     void                       *values;
+
+    union {
+        size_t                  size;
+    } u;
 } ngx_http_complex_value_t;
 
 
@@ -207,9 +211,13 @@ void ngx_http_script_flush_complex_value
     ngx_http_complex_value_t *val);
 ngx_int_t ngx_http_complex_value(ngx_http_request_t *r,
     ngx_http_complex_value_t *val, ngx_str_t *value);
+ngx_int_t ngx_http_complex_value_size(ngx_http_request_t *r,
+    ngx_http_complex_value_t *val, ngx_str_t *value, size_t *size);
 ngx_int_t ngx_http_compile_complex_value(ngx_http_compile_complex_value_t *ccv);
 char *ngx_http_set_complex_value_slot(ngx_conf_t *cf, ngx_command_t *cmd,
     void *conf);
+char *ngx_http_set_complex_value_size_slot(ngx_conf_t *cf, ngx_command_t *cmd,
+    void *conf);
 
 
 ngx_int_t ngx_http_test_predicates(ngx_http_request_t *r,
# HG changeset patch
# User Ruslan Ermilov <ru at nginx.com>
# Date 1544106977 -10800
#      Thu Dec 06 17:36:17 2018 +0300
# Node ID 586277aaab1be28cf53d2caa553a2bef11be2f34
# Parent  1a8c0a959f636b5eea0a4f42711af08591c23a99
Variables support in limit_rate and limit_rate_after (ticket #293).

diff --git a/src/http/ngx_http_core_module.c b/src/http/ngx_http_core_module.c
--- a/src/http/ngx_http_core_module.c
+++ b/src/http/ngx_http_core_module.c
@@ -479,7 +479,7 @@ static ngx_command_t  ngx_http_core_comm
     { ngx_string("limit_rate"),
       NGX_HTTP_MAIN_CONF|NGX_HTTP_SRV_CONF|NGX_HTTP_LOC_CONF|NGX_HTTP_LIF_CONF
                         |NGX_CONF_TAKE1,
-      ngx_conf_set_size_slot,
+      ngx_http_set_complex_value_size_slot,
       NGX_HTTP_LOC_CONF_OFFSET,
       offsetof(ngx_http_core_loc_conf_t, limit_rate),
       NULL },
@@ -487,7 +487,7 @@ static ngx_command_t  ngx_http_core_comm
     { ngx_string("limit_rate_after"),
       NGX_HTTP_MAIN_CONF|NGX_HTTP_SRV_CONF|NGX_HTTP_LOC_CONF|NGX_HTTP_LIF_CONF
                         |NGX_CONF_TAKE1,
-      ngx_conf_set_size_slot,
+      ngx_http_set_complex_value_size_slot,
       NGX_HTTP_LOC_CONF_OFFSET,
       offsetof(ngx_http_core_loc_conf_t, limit_rate_after),
       NULL },
@@ -1281,10 +1281,6 @@ ngx_http_update_location_config(ngx_http
         r->connection->tcp_nopush = NGX_TCP_NOPUSH_DISABLED;
     }
 
-    if (r->limit_rate == 0) {
-        r->limit_rate = clcf->limit_rate;
-    }
-
     if (clcf->handler) {
         r->content_handler = clcf->handler;
     }
@@ -3362,6 +3358,8 @@ ngx_http_core_create_loc_conf(ngx_conf_t
      *     clcf->exact_match = 0;
      *     clcf->auto_redirect = 0;
      *     clcf->alias = 0;
+     *     clcf->limit_rate = NULL;
+     *     clcf->limit_rate_after = NULL;
      *     clcf->gzip_proxied = 0;
      *     clcf->keepalive_disable = 0;
      */
@@ -3392,8 +3390,6 @@ ngx_http_core_create_loc_conf(ngx_conf_t
     clcf->send_timeout = NGX_CONF_UNSET_MSEC;
     clcf->send_lowat = NGX_CONF_UNSET_SIZE;
     clcf->postpone_output = NGX_CONF_UNSET_SIZE;
-    clcf->limit_rate = NGX_CONF_UNSET_SIZE;
-    clcf->limit_rate_after = NGX_CONF_UNSET_SIZE;
     clcf->keepalive_timeout = NGX_CONF_UNSET_MSEC;
     clcf->keepalive_header = NGX_CONF_UNSET;
     clcf->keepalive_requests = NGX_CONF_UNSET_UINT;
@@ -3581,6 +3577,14 @@ ngx_http_core_merge_loc_conf(ngx_conf_t 
     ngx_conf_merge_msec_value(conf->client_body_timeout,
                               prev->client_body_timeout, 60000);
 
+    if (conf->limit_rate == NULL) {
+        conf->limit_rate = prev->limit_rate;
+    }
+
+    if (conf->limit_rate_after == NULL) {
+        conf->limit_rate_after = prev->limit_rate_after;
+    }
+
     ngx_conf_merge_bitmask_value(conf->keepalive_disable,
                               prev->keepalive_disable,
                               (NGX_CONF_BITMASK_SET
@@ -3622,9 +3626,6 @@ ngx_http_core_merge_loc_conf(ngx_conf_t 
     ngx_conf_merge_size_value(conf->send_lowat, prev->send_lowat, 0);
     ngx_conf_merge_size_value(conf->postpone_output, prev->postpone_output,
                               1460);
-    ngx_conf_merge_size_value(conf->limit_rate, prev->limit_rate, 0);
-    ngx_conf_merge_size_value(conf->limit_rate_after, prev->limit_rate_after,
-                              0);
     ngx_conf_merge_msec_value(conf->keepalive_timeout,
                               prev->keepalive_timeout, 75000);
     ngx_conf_merge_sec_value(conf->keepalive_header,
diff --git a/src/http/ngx_http_core_module.h b/src/http/ngx_http_core_module.h
--- a/src/http/ngx_http_core_module.h
+++ b/src/http/ngx_http_core_module.h
@@ -350,13 +350,14 @@ struct ngx_http_core_loc_conf_s {
     size_t        client_body_buffer_size; /* client_body_buffer_size */
     size_t        send_lowat;              /* send_lowat */
     size_t        postpone_output;         /* postpone_output */
-    size_t        limit_rate;              /* limit_rate */
-    size_t        limit_rate_after;        /* limit_rate_after */
     size_t        sendfile_max_chunk;      /* sendfile_max_chunk */
     size_t        read_ahead;              /* read_ahead */
     size_t        subrequest_output_buffer_size;
                                            /* subrequest_output_buffer_size */
 
+    ngx_http_complex_value_t  *limit_rate; /* limit_rate */
+    ngx_http_complex_value_t  *limit_rate_after; /* limit_rate_after */
+
     ngx_msec_t    client_body_timeout;     /* client_body_timeout */
     ngx_msec_t    send_timeout;            /* send_timeout */
     ngx_msec_t    keepalive_timeout;       /* keepalive_timeout */
diff --git a/src/http/ngx_http_request.h b/src/http/ngx_http_request.h
--- a/src/http/ngx_http_request.h
+++ b/src/http/ngx_http_request.h
@@ -150,6 +150,9 @@
 #define NGX_HTTP_COPY_BUFFERED             0x04
 
 
+#define NGX_HTTP_LIMIT_RATE_OFF            NGX_MAX_SIZE_T_VALUE
+
+
 typedef enum {
     NGX_HTTP_INITING_REQUEST_STATE = 0,
     NGX_HTTP_READING_REQUEST_STATE,
diff --git a/src/http/ngx_http_upstream.c b/src/http/ngx_http_upstream.c
--- a/src/http/ngx_http_upstream.c
+++ b/src/http/ngx_http_upstream.c
@@ -2974,7 +2974,7 @@ ngx_http_upstream_send_response(ngx_http
         r->write_event_handler =
                              ngx_http_upstream_process_non_buffered_downstream;
 
-        r->limit_rate = 0;
+        r->limit_rate = NGX_HTTP_LIMIT_RATE_OFF;
 
         if (u->input_filter_init(u->input_filter_ctx) == NGX_ERROR) {
             ngx_http_upstream_finalize_request(r, u, NGX_ERROR);
diff --git a/src/http/ngx_http_write_filter_module.c b/src/http/ngx_http_write_filter_module.c
--- a/src/http/ngx_http_write_filter_module.c
+++ b/src/http/ngx_http_write_filter_module.c
@@ -48,6 +48,8 @@ ngx_int_t
 ngx_http_write_filter(ngx_http_request_t *r, ngx_chain_t *in)
 {
     off_t                      size, sent, nsent, limit;
+    size_t                     limit_rate;
+    ngx_str_t                  val;
     ngx_uint_t                 last, flush, sync;
     ngx_msec_t                 delay;
     ngx_chain_t               *cl, *ln, **ll, *chain;
@@ -250,9 +252,43 @@ ngx_http_write_filter(ngx_http_request_t
         return NGX_ERROR;
     }
 
-    if (r->limit_rate) {
-        if (r->limit_rate_after == 0) {
-            r->limit_rate_after = clcf->limit_rate_after;
+    if (r->limit_rate == 0) {
+
+        if (clcf->limit_rate
+            && ngx_http_complex_value_size(r, clcf->limit_rate, &val,
+                                           &limit_rate)
+               == NGX_OK)
+        {
+            if (limit_rate != (size_t) NGX_ERROR) {
+                r->limit_rate = limit_rate;
+
+            } else if (val.len) {
+                ngx_log_error(NGX_LOG_ERR, c->log, 0,
+                              "invalid \"limit_rate\" value \"%V\"", &val);
+            }
+        }
+
+        if (r->limit_rate == 0) {
+            r->limit_rate = NGX_HTTP_LIMIT_RATE_OFF;
+        }
+    }
+
+    if (r->limit_rate != NGX_HTTP_LIMIT_RATE_OFF) {
+
+        if (r->limit_rate_after == 0
+            && clcf->limit_rate_after
+            && ngx_http_complex_value_size(r, clcf->limit_rate_after, &val,
+                                           &limit_rate)
+               == NGX_OK)
+        {
+            if (limit_rate != (size_t) NGX_ERROR) {
+                r->limit_rate_after = limit_rate;
+
+            } else if (val.len) {
+                ngx_log_error(NGX_LOG_ERR, c->log, 0,
+                              "invalid \"limit_rate_after\" value \"%V\"",
+                              &val);
+            }
         }
 
         limit = (off_t) r->limit_rate * (ngx_time() - r->start_sec + 1)
@@ -293,7 +329,7 @@ ngx_http_write_filter(ngx_http_request_t
         return NGX_ERROR;
     }
 
-    if (r->limit_rate) {
+    if (r->limit_rate != NGX_HTTP_LIMIT_RATE_OFF) {
 
         nsent = c->sent;
 


More information about the nginx-devel mailing list