[PATCH 3 of 3] SSI: Implement #flastmod SSI command

[Maxim Dounin]

Hello!

On Fri, Mar 10, 2017 at 12:10:34PM +0300, Matwey V. Kornilov wrote:

> # HG changeset patch
> # User Matwey V. Kornilov <matwey.kornilov at gmail.com>
> # Date 1489136594 -10800
> #      Fri Mar 10 12:03:14 2017 +0300
> # Branch fsize
> # Node ID 7f0c8192573f8db513be169ba355c117cc80e146
> # Parent  35527b6bd4d84e41e030972b770e75ef583cf0f5
> SSI: Implement #flastmod SSI command

Style, should be:

SSI: implemented "flastmod" SSI command.

> 
> diff -r 35527b6bd4d8 -r 7f0c8192573f src/http/modules/ngx_http_ssi_filter_module.c
> --- a/src/http/modules/ngx_http_ssi_filter_module.c	Wed Feb 08 20:58:49 2017 +0300
> +++ b/src/http/modules/ngx_http_ssi_filter_module.c	Fri Mar 10 12:03:14 2017 +0300
> @@ -93,6 +93,8 @@
>      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_flastmod_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,
> @@ -311,6 +313,8 @@
>                         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("flastmod"), 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,

This probably needs better names to express the fact that 
ngx_http_ssi_fsize / ngx_http_ssi_fsize_params are used for both 
fsize and flastmod commands.

Alternatively, it may worth using different functions here, and 
introducing some internal function used by both of them instead, 
with handler passed as an additional argument.  May be even common 
one for include/fsize/flastmod.

> @@ -2275,14 +2279,14 @@
>  
>      if (uri && file) {
>          ngx_log_error(NGX_LOG_ERR, r->connection->log, 0,
> -                      "fsize may be either virtual=\"%V\" or file=\"%V\"",
> -                      uri, file);
> +                      "%V may be either virtual=\"%V\" or file=\"%V\"",
> +                      ctx->command, 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");
> +                      "no parameter in \"%V\" SSI command", ctx->command);
>          return NGX_HTTP_SSI_ERROR;
>      }
>  
> @@ -2311,7 +2315,11 @@
>          return NGX_ERROR;
>      }
>  
> -    psr->handler = ngx_http_ssi_fsize_output;
> +    if (ctx->command.len == sizeof("fsize") - 1) {
> +        psr->handler = ngx_http_ssi_fsize_output;
> +    } else if (ctx->command.len == sizeof("flastmod") - 1) {
> +        psr->handler = ngx_http_ssi_flastmod_output;
> +    }
>      psr->data = ctx;

Style, there should be more empty lines here.

>  
>      if (ngx_http_subrequest(r, uri, &args, &sr, psr, flags) != NGX_OK) {
> @@ -2380,6 +2388,56 @@
>  
>  
>  static ngx_int_t
> +ngx_http_ssi_flastmod_output(ngx_http_request_t *r, void *data, ngx_int_t rc)
> +{
> +    ngx_chain_t        *out;
> +    ngx_buf_t          *b;
> +    u_char             *p;
> +    ngx_http_ssi_ctx_t *ctx;
> +    ngx_str_t          *timefmt;
> +    ngx_str_t           timestr;
> +
> +    ctx = data;
> +    timefmt = &ctx->timefmt;
> +
> +    if (r->request_output) {
> +        return rc;
> +    }
> +
> +    if ((r->headers_out.last_modified_time == -1)
> +        || (ngx_http_ssi_format_time(r, &timestr,
> +                                     r->headers_out.last_modified_time,
> +                                     timefmt, 0) != NGX_OK))

This certainly needs to be written in a more readable form.

> +    {
> +        b = ngx_create_temp_buf(r->pool, 1);
> +        if (b == NULL) {
> +            return NGX_ERROR;
> +        }
> +        p = b->start;
> +        b->last = ngx_sprintf(p, "-");

See previous comments on whether "-" is appropriate or not.

> +    } else {
> +        b = ngx_calloc_buf(r->pool);
> +        if (b == NULL) {
> +            return NGX_ERROR;
> +        }
> +        b->memory = 1;
> +        b->pos = timestr.data;
> +        b->last = b->pos + timestr.len;
> +    }
> +
> +    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);
> +}
> +
> +
> +static ngx_int_t
>  ngx_http_ssi_echo(ngx_http_request_t *r, ngx_http_ssi_ctx_t *ctx,
>      ngx_str_t **params)
>  {

-- 
Maxim Dounin
http://nginx.org/