[PATCH 1 of 3] SSI: Implement #fsize SSI command
Maxim Dounin
mdounin at mdounin.ru
Thu Apr 13 17:24:07 UTC 2017
Hello!
Sorry for late response.
See comments below.
On Fri, Mar 10, 2017 at 12:10:32PM +0300, Matwey V. Kornilov wrote:
> # HG changeset patch
> # User Matwey V. Kornilov <matwey.kornilov at gmail.com>
> # Date 1489136573 -10800
> # Fri Mar 10 12:02:53 2017 +0300
> # Branch fsize
> # Node ID a682e88cdcddb04905814cdfacde3df9ebc2b48b
> # Parent 640f035293959b2d4b0ba5939d954bc517f57f77
> SSI: Implement #fsize SSI command
Style, should be:
SSI: implemented "fsize" SSI command.
>
> diff -r 640f03529395 -r a682e88cdcdd src/http/modules/ngx_http_ssi_filter_module.c
> --- a/src/http/modules/ngx_http_ssi_filter_module.c Fri Jan 27 19:06:35 2017 +0300
> +++ b/src/http/modules/ngx_http_ssi_filter_module.c Fri Mar 10 12:02:53 2017 +0300
> @@ -89,6 +89,10 @@
> ngx_int_t rc);
> static ngx_int_t ngx_http_ssi_set_variable(ngx_http_request_t *r, void *data,
> ngx_int_t rc);
> +static ngx_int_t ngx_http_ssi_fsize(ngx_http_request_t *r,
> + ngx_http_ssi_ctx_t *ctx, ngx_str_t **params);
> +static ngx_int_t ngx_http_ssi_fsize_output(ngx_http_request_t *r, void *data,
> + ngx_int_t rc);
> static ngx_int_t ngx_http_ssi_echo(ngx_http_request_t *r,
> ngx_http_ssi_ctx_t *ctx, ngx_str_t **params);
> static ngx_int_t ngx_http_ssi_config(ngx_http_request_t *r,
> @@ -211,6 +215,7 @@
>
>
> static u_char ngx_http_ssi_string[] = "<!--";
> +static u_char ngx_http_ssi_si_prefix[] = " kMGTPEZY";
>
> static ngx_str_t ngx_http_ssi_none = ngx_string("(none)");
> static ngx_str_t ngx_http_ssi_timefmt = ngx_string("%A, %d-%b-%Y %H:%M:%S %Z");
> @@ -223,12 +228,16 @@
> #define NGX_HTTP_SSI_INCLUDE_SET 3
> #define NGX_HTTP_SSI_INCLUDE_STUB 4
>
> +#define NGX_HTTP_SSI_FSIZE_VIRTUAL 0
> +#define NGX_HTTP_SSI_FSIZE_FILE 1
> +
> #define NGX_HTTP_SSI_ECHO_VAR 0
> #define NGX_HTTP_SSI_ECHO_DEFAULT 1
> #define NGX_HTTP_SSI_ECHO_ENCODING 2
>
> #define NGX_HTTP_SSI_CONFIG_ERRMSG 0
> #define NGX_HTTP_SSI_CONFIG_TIMEFMT 1
> +#define NGX_HTTP_SSI_CONFIG_SIZEFMT 2
>
> #define NGX_HTTP_SSI_SET_VAR 0
> #define NGX_HTTP_SSI_SET_VALUE 1
> @@ -248,6 +257,13 @@
> };
>
>
> +static ngx_http_ssi_param_t ngx_http_ssi_fsize_params[] = {
> + { ngx_string("virtual"), NGX_HTTP_SSI_FSIZE_VIRTUAL, 0, 0 },
> + { ngx_string("file"), NGX_HTTP_SSI_FSIZE_FILE, 0, 0 },
> + { ngx_null_string, 0, 0, 0 }
> +};
> +
> +
> static ngx_http_ssi_param_t ngx_http_ssi_echo_params[] = {
> { ngx_string("var"), NGX_HTTP_SSI_ECHO_VAR, 1, 0 },
> { ngx_string("default"), NGX_HTTP_SSI_ECHO_DEFAULT, 0, 0 },
> @@ -259,6 +275,7 @@
> static ngx_http_ssi_param_t ngx_http_ssi_config_params[] = {
> { ngx_string("errmsg"), NGX_HTTP_SSI_CONFIG_ERRMSG, 0, 0 },
> { ngx_string("timefmt"), NGX_HTTP_SSI_CONFIG_TIMEFMT, 0, 0 },
> + { ngx_string("sizefmt"), NGX_HTTP_SSI_CONFIG_SIZEFMT, 0, 0 },
> { ngx_null_string, 0, 0, 0 }
> };
>
> @@ -290,6 +307,8 @@
> static ngx_http_ssi_command_t ngx_http_ssi_commands[] = {
> { ngx_string("include"), ngx_http_ssi_include,
> ngx_http_ssi_include_params, 0, 0, 1 },
> + { ngx_string("fsize"), ngx_http_ssi_fsize,
> + ngx_http_ssi_fsize_params, 0, 0, 1 },
> { ngx_string("echo"), ngx_http_ssi_echo,
> ngx_http_ssi_echo_params, 0, 0, 0 },
> { ngx_string("config"), ngx_http_ssi_config,
> @@ -2239,6 +2258,124 @@
> return rc;
> }
>
> +static ngx_int_t
Style: should be 2 empty lines between functions.
> +ngx_http_ssi_fsize(ngx_http_request_t *r, ngx_http_ssi_ctx_t *ctx,
> + ngx_str_t **params)
> +{
> + ngx_int_t rc;
> + ngx_str_t *uri, *file, args;
> + ngx_uint_t flags;
> + ngx_http_request_t *sr;
> + ngx_http_post_subrequest_t *psr;
> +
> + uri = params[NGX_HTTP_SSI_FSIZE_VIRTUAL];
> + file = params[NGX_HTTP_SSI_FSIZE_FILE];
> +
> + if (uri && file) {
> + ngx_log_error(NGX_LOG_ERR, r->connection->log, 0,
> + "fsize may be either virtual=\"%V\" or file=\"%V\"",
> + uri, file);
> + return NGX_HTTP_SSI_ERROR;
> + }
> +
> + if (uri == NULL && file == NULL) {
> + ngx_log_error(NGX_LOG_ERR, r->connection->log, 0,
> + "no parameter in \"fsize\" SSI command");
> + return NGX_HTTP_SSI_ERROR;
> + }
> +
> + if (uri == NULL) {
> + uri = file;
It may be a good idea to add "wait" parameter, much like for the
"include" command. And using using "wait" logic for "fsize file"
(in contrast to "fsize virtual"), much like we do so for
the "include" command.
> + }
> +
> + rc = ngx_http_ssi_evaluate_string(r, ctx, uri, NGX_HTTP_SSI_ADD_PREFIX);
> +
> + if (rc != NGX_OK) {
> + return rc;
> + }
> +
> + ngx_log_debug1(NGX_LOG_DEBUG_HTTP, r->connection->log, 0,
> + "ssi fsize: \"%V\"", uri);
> +
> + ngx_str_null(&args);
> + flags = NGX_HTTP_LOG_UNSAFE;
> +
> + if (ngx_http_parse_unsafe_uri(r, uri, &args, &flags) != NGX_OK) {
> + return NGX_HTTP_SSI_ERROR;
> + }
> +
> + psr = ngx_palloc(r->pool, sizeof(ngx_http_post_subrequest_t));
> + if (psr == NULL) {
> + return NGX_ERROR;
> + }
> +
> + psr->handler = ngx_http_ssi_fsize_output;
> + psr->data = ctx;
> +
> + if (ngx_http_subrequest(r, uri, &args, &sr, psr, flags) != NGX_OK) {
> + return NGX_HTTP_SSI_ERROR;
> + }
> + sr->header_only = 1;
Style: should be an empty line between "}" and the following code.
It may be a good idea to use HEAD method here, to minimize load on
the upstream servers, if any. YMMV.
> +
> + return NGX_OK;
> +}
> +
> +
> +static ngx_int_t
> +ngx_http_ssi_fsize_output(ngx_http_request_t *r, void *data, ngx_int_t rc)
> +{
> + ngx_chain_t *out;
> + ngx_buf_t *b;
> + u_char *p;
> + size_t len, i;
> + ngx_http_ssi_ctx_t *ctx;
> + unsigned sizefmt_bytes;
Style, variables should be sorted based on the type length, and
there should be at least two spaces between the type and the
variable name:
u_char *p;
size_t len, i;
unsigned sizefmt_bytes;
ngx_buf_t *b;
ngx_chain_t *out;
ngx_http_ssi_ctx_t *ctx;
> +
> + ctx = data;
> + sizefmt_bytes = ctx->sizefmt_bytes;
A shorter name may be an option here, something like ctx->bytes
and bytes respectively. Alternatively, "exact_size" may be used
as in the autoindex module.
> +
> + if (r->request_output) {
> + return rc;
> + }
> +
It may make sense to add a debugging log here, similar to one used
in ngx_http_ssi_stub_output():
ngx_log_debug2(NGX_LOG_DEBUG_HTTP, r->connection->log, 0,
"ssi fsize output: \"%V?%V\"", &r->uri, &r->args);
> + b = ngx_create_temp_buf(r->pool, NGX_OFF_T_LEN + 2);
> + if (b == NULL) {
> + return NGX_ERROR;
> + }
> + p = b->start;
Style, should be an empty line after "}".
> +
> + if (r->headers_out.content_length_n == -1) {
> + b->last = ngx_sprintf(p, "-");
> + } else {
Style, should be an empty line before "} else {".
It is also not clear if "-" is a correct value here. As far as I
can tell, for example Apache will print an SSI error in most
relevant cases.
> + b->last = ngx_sprintf(p, "%O", r->headers_out.content_length_n);
> + len = b->last - p;
> +
> + if (!sizefmt_bytes && r->headers_out.content_length_n >= 1000) {
> + i = len % 3;
Each kilobyte contains 1024 bytes, not 1000.
Take a look at the autoindex module for the code which also
formats a size. It may be also a good idea to introduce a generic
function and use it both in autoindex and here; not sure though, a
simple copy may be a better option for now.
> +
> + if (i == 0) {
> + p[3] = ngx_http_ssi_si_prefix[len / 3 - 1];
> + b->last = p + 4;
> + } else {
> + p[5] = ngx_http_ssi_si_prefix[len / 3];
> + ngx_memmove(p + i + 1, p + i, 4 - i);
> + p[i] = '.';
> + b->last = p + 6;
> + }
> + }
> + }
> +
> + out = ngx_alloc_chain_link(r->pool);
> + if (out == NULL) {
> + return NGX_ERROR;
> + }
> +
> + out->buf = b;
> + out->next = NULL;
> +
> + return ngx_http_output_filter(r, out);
Note:
This uses body output in a subrequest with r->header_only set.
While this currently works, it looks like a hack and I suspect it
may be broken in the future.
Not sure how to solve this properly though. The only "clean"
alternative I see is to always wait for such subrequests and do
the output in the original request context. No sure if it is
better than the current approach though, as this will imply
waiting for all such subrequests and won't allow parallel requests
to upstream servers.
> +}
> +
>
> static ngx_int_t
> ngx_http_ssi_echo(ngx_http_request_t *r, ngx_http_ssi_ctx_t *ctx,
> @@ -2400,6 +2537,24 @@
> ctx->errmsg = *value;
> }
>
> + value = params[NGX_HTTP_SSI_CONFIG_SIZEFMT];
> +
> + if (value) {
> + if (value->len == 5
> + && ngx_strncasecmp(value->data, (u_char *) "bytes", 5) == 0)
> + {
> + ctx->sizefmt_bytes = 1;
> + } else if (value->len == 6
> + && ngx_strncasecmp(value->data, (u_char *) "abbrev", 6) == 0)
> + {
> + ctx->sizefmt_bytes = 0;
> + } else {
Style: there should be empty lines before "} else ...".
> + ngx_log_error(NGX_LOG_ERR, r->connection->log, 0,
> + "#config sizefmt may be either \"bytes\" or \"abbrev\"");
SSI module uses different form of errors, and it is a good idea to
be consistent with other similar error messages. General form is:
"...what's wrong... in "name" SSI command".
In this particular case, something like
ngx_log_error(NGX_LOG_ERR, r->connection->log, 0,
"unknown size format \"%V\" "
"in \"config\" SSI command",
value);
should be appropriate.
> + return NGX_HTTP_SSI_ERROR;
> + }
> + }
> +
> return NGX_OK;
> }
>
> diff -r 640f03529395 -r a682e88cdcdd src/http/modules/ngx_http_ssi_filter_module.h
> --- a/src/http/modules/ngx_http_ssi_filter_module.h Fri Jan 27 19:06:35 2017 +0300
> +++ b/src/http/modules/ngx_http_ssi_filter_module.h Fri Mar 10 12:02:53 2017 +0300
> @@ -76,6 +76,7 @@
> unsigned block:1;
> unsigned output:1;
> unsigned output_chosen:1;
> + unsigned sizefmt_bytes:1;
>
> ngx_http_request_t *wait;
> void *value_buf;
--
Maxim Dounin
http://nginx.org/
More information about the nginx-devel
mailing list