Hello!
On Fri, Jun 02, 2017 at 08:33:47PM -0700, Piotr Sikora via nginx-devel wrote:
> # HG changeset patch
> # User Piotr Sikora <piotrsik...@google.com>
> # Date 1490351854 25200
> # Fri Mar 24 03:37:34 2017 -0700
> # Node ID acdc80c0d4ef8aa2519e2882ff1a3bd4a316ad81
> # Parent 8d74ff6c2015180f5c1f399f492214d7d0a52b3f
> Headers filter: added "add_trailer" directive.
>
> Trailers added using this directive are evaluated after response body
> is processed by output filters (but before it's written to the wire),
> so it's possible to use variables calculated from the response body
> as the trailer value.
>
> Signed-off-by: Piotr Sikora <piotrsik...@google.com>
Overall looks good, see some comments below.
[...]
> @@ -206,11 +223,103 @@ ngx_http_headers_filter(ngx_http_request
> }
> }
>
> + if (conf->trailers) {
> + h = conf->trailers->elts;
> + for (i = 0; i < conf->trailers->nelts; i++) {
> +
> + if (!safe_status && !h[i].always) {
> + continue;
> + }
> +
> + if (h[i].value.value.len) {
> + r->expect_trailers = 1;
> + break;
> + }
It doesn't look like "if (h[i].value.value.len)" is needed here.
It is either true, or the "add_trailer" directive is nop and we
already know this while parsing the configuration.
- if (h[i].value.value.len) {
- r->expect_trailers = 1;
- break;
- }
+ r->expect_trailers = 1;
+ break;
[...]
> @@ -741,3 +858,63 @@ ngx_http_headers_add(ngx_conf_t *cf, ngx
>
> return NGX_CONF_OK;
> }
> +
> +
> +static char *
> +ngx_http_headers_add_trailer(ngx_conf_t *cf, ngx_command_t *cmd, void *conf)
> +{
> + ngx_http_headers_conf_t *hcf = conf;
> +
> + ngx_str_t *value;
> + ngx_http_header_val_t *hv;
> + ngx_http_compile_complex_value_t ccv;
> +
> + value = cf->args->elts;
> +
> + if (hcf->trailers == NULL) {
> + hcf->trailers = ngx_array_create(cf->pool, 1,
> + sizeof(ngx_http_header_val_t));
> + if (hcf->trailers == NULL) {
> + return NGX_CONF_ERROR;
> + }
> + }
> +
> + hv = ngx_array_push(hcf->trailers);
> + if (hv == NULL) {
> + return NGX_CONF_ERROR;
> + }
[...]
It looks like the ngx_http_headers_add_trailer() function is
almost exact copy of the ngx_http_headers_add(). It might worth
to use single function to handle both directives.
diff --git a/src/http/modules/ngx_http_headers_filter_module.c
b/src/http/modules/ngx_http_headers_filter_module.c
--- a/src/http/modules/ngx_http_headers_filter_module.c
+++ b/src/http/modules/ngx_http_headers_filter_module.c
@@ -73,8 +73,6 @@ static char *ngx_http_headers_expires(ng
void *conf);
static char *ngx_http_headers_add(ngx_conf_t *cf, ngx_command_t *cmd,
void *conf);
-static char *ngx_http_headers_add_trailer(ngx_conf_t *cf, ngx_command_t *cmd,
- void *conf);
static ngx_http_set_header_t ngx_http_set_headers[] = {
@@ -108,15 +106,15 @@ static ngx_command_t ngx_http_headers_f
|NGX_CONF_TAKE23,
ngx_http_headers_add,
NGX_HTTP_LOC_CONF_OFFSET,
- 0,
+ offsetof(ngx_http_headers_conf_t, headers),
NULL },
{ ngx_string("add_trailer"),
NGX_HTTP_MAIN_CONF|NGX_HTTP_SRV_CONF|NGX_HTTP_LOC_CONF|NGX_HTTP_LIF_CONF
|NGX_CONF_TAKE23,
- ngx_http_headers_add_trailer,
+ ngx_http_headers_add,
NGX_HTTP_LOC_CONF_OFFSET,
- 0,
+ offsetof(ngx_http_headers_conf_t, trailers),
NULL },
ngx_null_command
@@ -231,10 +229,8 @@ ngx_http_headers_filter(ngx_http_request
continue;
}
- if (h[i].value.value.len) {
- r->expect_trailers = 1;
- break;
- }
+ r->expect_trailers = 1;
+ break;
}
}
@@ -791,42 +787,49 @@ ngx_http_headers_add(ngx_conf_t *cf, ngx
{
ngx_http_headers_conf_t *hcf = conf;
- ngx_str_t *value;
- ngx_uint_t i;
- ngx_http_header_val_t *hv;
- ngx_http_set_header_t *set;
- ngx_http_compile_complex_value_t ccv;
+ ngx_str_t *value;
+ ngx_uint_t i;
+ ngx_array_t **headers;
+ ngx_http_header_val_t *hv;
+ ngx_http_set_header_t *set;
+ ngx_http_compile_complex_value_t ccv;
value = cf->args->elts;
- if (hcf->headers == NULL) {
- hcf->headers = ngx_array_create(cf->pool, 1,
- sizeof(ngx_http_header_val_t));
- if (hcf->headers == NULL) {
+ headers = (ngx_array_t **) ((char *) hcf + cmd->offset);
+
+ if (*headers == NULL) {
+ *headers = ngx_array_create(cf->pool, 1,
+ sizeof(ngx_http_header_val_t));
+ if (*headers == NULL) {
return NGX_CONF_ERROR;
}
}
- hv = ngx_array_push(hcf->headers);
+ hv = ngx_array_push(*headers);
if (hv == NULL) {
return NGX_CONF_ERROR;
}
hv->key = value[1];
- hv->handler = ngx_http_add_header;
+ hv->handler = NULL;
hv->offset = 0;
hv->always = 0;
- set = ngx_http_set_headers;
- for (i = 0; set[i].name.len; i++) {
- if (ngx_strcasecmp(value[1].data, set[i].name.data) != 0) {
- continue;
+ if (headers == &hcf->headers) {
+ hv->handler = ngx_http_add_header;
+
+ set = ngx_http_set_headers;
+ for (i = 0; set[i].name.len; i++) {
+ if (ngx_strcasecmp(value[1].data, set[i].name.data) != 0) {
+ continue;
+ }
+
+ hv->offset = set[i].offset;
+ hv->handler = set[i].handler;
+
+ break;
}
-
- hv->offset = set[i].offset;
- hv->handler = set[i].handler;
-
- break;
}
if (value[2].len == 0) {
@@ -858,63 +861,3 @@ ngx_http_headers_add(ngx_conf_t *cf, ngx
return NGX_CONF_OK;
}
-
-
-static char *
-ngx_http_headers_add_trailer(ngx_conf_t *cf, ngx_command_t *cmd, void *conf)
-{
- ngx_http_headers_conf_t *hcf = conf;
-
- ngx_str_t *value;
- ngx_http_header_val_t *hv;
- ngx_http_compile_complex_value_t ccv;
-
- value = cf->args->elts;
-
- if (hcf->trailers == NULL) {
- hcf->trailers = ngx_array_create(cf->pool, 1,
- sizeof(ngx_http_header_val_t));
- if (hcf->trailers == NULL) {
- return NGX_CONF_ERROR;
- }
- }
-
- hv = ngx_array_push(hcf->trailers);
- if (hv == NULL) {
- return NGX_CONF_ERROR;
- }
-
- hv->key = value[1];
- hv->handler = NULL;
- hv->offset = 0;
- hv->always = 0;
-
- if (value[2].len == 0) {
- ngx_memzero(&hv->value, sizeof(ngx_http_complex_value_t));
-
- } else {
- ngx_memzero(&ccv, sizeof(ngx_http_compile_complex_value_t));
-
- ccv.cf = cf;
- ccv.value = &value[2];
- ccv.complex_value = &hv->value;
-
- if (ngx_http_compile_complex_value(&ccv) != NGX_OK) {
- return NGX_CONF_ERROR;
- }
- }
-
- if (cf->args->nelts == 3) {
- return NGX_CONF_OK;
- }
-
- if (ngx_strcmp(value[3].data, "always") != 0) {
- ngx_conf_log_error(NGX_LOG_EMERG, cf, 0,
- "invalid parameter \"%V\"", &value[3]);
- return NGX_CONF_ERROR;
- }
-
- hv->always = 1;
-
- return NGX_CONF_OK;
-}
--
Maxim Dounin
http://nginx.org/
_______________________________________________
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel
