limit_rate_after support variables

[Ruslan Ermilov]

Hi,

On Tue, Aug 14, 2018 at 10:22:14AM +0200, Miroslav Novy wrote:
> # HG changeset patch
> # User Miroslav Nový <miranovy at gmail.com>
> # Date 1534234559 0
> #      Tue Aug 14 08:15:59 2018 +0000
> # Node ID 1a8327b50f7844cbe68226f54de60632189327f4
> # Parent  70c6b08973a02551612da4a4273757dc77c70ae2
> limit_rate_after support variables
> 
> Example of use:
> location / {
>     root /var/www/default/;
>             index  index.html index.htm;
> 
>     set $my_limit_rate_after 2m;
> 
>     limit_rate_after $my_limit_rate_after;
>     limit_rate 2k;
> 
>     access_by_lua_block {
> ngx.var.my_limit_rate_after = '10m'
>     }
> 
>         }
> 
> diff -r 70c6b08973a0 -r 1a8327b50f78 src/http/ngx_http_core_module.c
> --- a/src/http/ngx_http_core_module.c Fri Aug 10 21:54:46 2018 +0300
> +++ b/src/http/ngx_http_core_module.c Tue Aug 14 08:15:59 2018 +0000
> @@ -487,7 +487,7 @@
>      { 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_slot,
>        NGX_HTTP_LOC_CONF_OFFSET,
>        offsetof(ngx_http_core_loc_conf_t, limit_rate_after),
>        NULL },
> @@ -3364,6 +3364,7 @@
>       *     clcf->alias = 0;
>       *     clcf->gzip_proxied = 0;
>       *     clcf->keepalive_disable = 0;
> +     *     clcf->limit_rate_after = NULL;
>       */
> 
>      clcf->client_max_body_size = NGX_CONF_UNSET;
> @@ -3393,7 +3394,6 @@
>      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;
> @@ -3623,8 +3623,8 @@
>      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_ptr_value(conf->limit_rate_after,
> prev->limit_rate_after,
> +                              NULL);

This won't work because create_loc_conf() initialized clcf->limit_rate_after
to NULL instead of NGX_CONF_UNSET_PTR.  I suggest using a simple "== NULL"
check here.

>      ngx_conf_merge_msec_value(conf->keepalive_timeout,
>                                prev->keepalive_timeout, 75000);
>      ngx_conf_merge_sec_value(conf->keepalive_header,
> diff -r 70c6b08973a0 -r 1a8327b50f78 src/http/ngx_http_core_module.h
> --- a/src/http/ngx_http_core_module.h Fri Aug 10 21:54:46 2018 +0300
> +++ b/src/http/ngx_http_core_module.h Tue Aug 14 08:15:59 2018 +0000
> @@ -351,7 +351,7 @@
>      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 */
> +    ngx_http_complex_value_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;
> diff -r 70c6b08973a0 -r 1a8327b50f78 src/http/ngx_http_write_filter_module.c
> --- a/src/http/ngx_http_write_filter_module.c Fri Aug 10 21:54:46 2018 +0300
> +++ b/src/http/ngx_http_write_filter_module.c Tue Aug 14 08:15:59 2018 +0000
> @@ -220,7 +220,26 @@
> 
>      if (r->limit_rate) {
>          if (r->limit_rate_after == 0) {
> -            r->limit_rate_after = clcf->limit_rate_after;
> +            r->limit_rate_after = 0;

This assignment is pointless.

> +
> +            if (clcf->limit_rate_after != NULL) {

You can omit the "!= NULL" part of the check.

> +                ngx_str_t res;
> +                size_t    st;

There are style issues here, please see
http://nginx.org/en/docs/dev/development_guide.html#code_style

Also, I suggest renaming "st" to "s" here, and changing its type to
ssize_t.

> +
> +                if (ngx_http_complex_value(r, clcf->limit_rate_after, &res)
> +                    != NGX_OK)
> +                {
> +                    return NGX_ERROR;
> +                }
> +
> +                st = ngx_parse_size(&res);
> +                if (st != (size_t) NGX_ERROR) {
> +                    r->limit_rate_after = st;
> +                } else {
> +                    ngx_log_error(NGX_LOG_ALERT, c->log, 0,
> +                                  "limit_rate_after has bad value");
> +                }
> +            }
>          }
> 
>          limit = (off_t) r->limit_rate * (ngx_time() - r->start_sec + 1)

P.S. Please find another MUA that doesn't break text attachments.


-- 
Ruslan Ermilov
Assume stupidity not malice