Patch attached
On Sun, Jan 22, 2017 at 4:07 PM Alon Blayer-Gat <alon.blayer...@gmail.com>
wrote:
> Sure. Thanks for the feedback. I made is simpler.
>
> evil 'if in location' removed :). I also removed the 'gunzip types;'
> option.
> And now we now only have 'gunzip off|on|always'
>
> But then again, with 'always', one must specify with 'gunzip_types' the
> mime types to always gunzip.
> The rational is that if we want to always gunzip, then it's probably not
> because the client does not support it but rather because we would like to
> modify the response. In which case, it is needed only for specific content
> types.
> Default mime type is text/html ( similar to gzip_types).
>
> I thought of your suggestion of making a more generic mechanism to be used
> by modules directly. It would require modifications in existing modules to
> make the feature usable.
> This patch suggests a simple change for the benefit of existing
> text-related body-filter modules.
>
> Hope it makes sense.
> Patch attached.
>
> Thanks,
>
> /Alon
>
>
> On Wed, Dec 7, 2016 at 5:58 PM Maxim Dounin <mdou...@mdounin.ru> wrote:
>
> > Hello!
> >
> > On Sun, Nov 27, 2016 at 02:27:56PM +0200, Alon Blayer-Gat wrote:
> >
> > > Hi,
> > >
> > > 1) 'gunzip always' option will gunzip even if the client supports it.
> > > 2) 'gunzip types', like 'always' but only for file types specified
> > > with 'gunzip_types <mime-types>'
> > > 3) Allow gunzip and gunzip_types directives within "if in location"
> > > block (rewrite phase condition).
> > >
> > > The suggested changes are needed, mainly, to allow dynamic
> > > modification of compressed response (e.g. with the 'sub_filter'
> > > module)
> > > 'types' and 'if in location' may allow a more selective operation.
> >
> > No, thanks.
> >
> > "If in location" is evil, don't even try to suggest patches to
> > allow directives in the "if in location" context.
> >
> > As for other changes - I can't say I like them as well. We may
> > consider something as simple as "gunzip always", but additional
> > types filter certainly looks like an overkill. Rather it should
> > be some more generic mechanism to require gunzipping, may be
> > useable by modules directly.
> >
> > [...]
> >
> > --
> > Maxim Dounin
> > http://nginx.org/
> > _______________________________________________
> > nginx-devel mailing list
> > nginx-devel@nginx.org
> > http://mailman.nginx.org/mailman/listinfo/nginx-devel
> >
> _______________________________________________
> nginx-devel mailing list
> nginx-devel@nginx.org
> http://mailman.nginx.org/mailman/listinfo/nginx-devel
--- .dist/openresty/openresty-1.11.2.2/bundle/nginx-1.11.2/src/http/modules/ngx_http_gunzip_filter_module.c 2016-07-18 05:19:50.000000000 +0300
+++ ../ngx_http_gunzip_filter_module/ngx_http_gunzip_filter_module.c 2017-01-19 17:44:08.109134808 +0200
@@ -13,9 +13,16 @@
#include <zlib.h>
+#define NGX_HTTP_GUNZIP_OFF 0
+#define NGX_HTTP_GUNZIP_ON 1
+#define NGX_HTTP_GUNZIP_ALWAYS 2
+
+
typedef struct {
- ngx_flag_t enable;
+ ngx_uint_t enable;
ngx_bufs_t bufs;
+ ngx_hash_t types;
+ ngx_array_t *types_keys;
} ngx_http_gunzip_conf_t;
@@ -62,14 +69,22 @@
void *parent, void *child);
+static ngx_conf_enum_t ngx_http_gunzip[] = {
+ { ngx_string("off"), NGX_HTTP_GUNZIP_OFF },
+ { ngx_string("on"), NGX_HTTP_GUNZIP_ON },
+ { ngx_string("always"), NGX_HTTP_GUNZIP_ALWAYS },
+ { ngx_null_string, 0 }
+};
+
+
static ngx_command_t ngx_http_gunzip_filter_commands[] = {
{ ngx_string("gunzip"),
- NGX_HTTP_MAIN_CONF|NGX_HTTP_SRV_CONF|NGX_HTTP_LOC_CONF|NGX_CONF_FLAG,
- ngx_conf_set_flag_slot,
+ NGX_HTTP_MAIN_CONF|NGX_HTTP_SRV_CONF|NGX_HTTP_LOC_CONF|NGX_CONF_TAKE1,
+ ngx_conf_set_enum_slot,
NGX_HTTP_LOC_CONF_OFFSET,
offsetof(ngx_http_gunzip_conf_t, enable),
- NULL },
+ &ngx_http_gunzip },
{ ngx_string("gunzip_buffers"),
NGX_HTTP_MAIN_CONF|NGX_HTTP_SRV_CONF|NGX_HTTP_LOC_CONF|NGX_CONF_TAKE2,
@@ -78,6 +93,13 @@
offsetof(ngx_http_gunzip_conf_t, bufs),
NULL },
+ { ngx_string("gunzip_types"),
+ NGX_HTTP_MAIN_CONF|NGX_HTTP_SRV_CONF|NGX_HTTP_LOC_CONF|NGX_CONF_1MORE,
+ ngx_http_types_slot,
+ NGX_HTTP_LOC_CONF_OFFSET,
+ offsetof(ngx_http_gunzip_conf_t, types_keys),
+ &ngx_http_html_default_types[0] },
+
ngx_null_command
};
@@ -126,10 +148,9 @@
conf = ngx_http_get_module_loc_conf(r, ngx_http_gunzip_filter_module);
/* TODO support multiple content-codings */
- /* TODO always gunzip - due to configuration or module request */
/* TODO ignore content encoding? */
- if (!conf->enable
+ if (conf->enable == NGX_HTTP_GUNZIP_OFF
|| r->headers_out.content_encoding == NULL
|| r->headers_out.content_encoding->value.len != 4
|| ngx_strncasecmp(r->headers_out.content_encoding->value.data,
@@ -140,13 +161,20 @@
r->gzip_vary = 1;
- if (!r->gzip_tested) {
- if (ngx_http_gzip_ok(r) == NGX_OK) {
- return ngx_http_next_header_filter(r);
+ if (conf->enable == NGX_HTTP_GUNZIP_ON) {
+ if (!r->gzip_tested) {
+ if (ngx_http_gzip_ok(r) == NGX_OK) {
+ return ngx_http_next_header_filter(r);
+ }
+
+ } else if (r->gzip_ok) {
+ return ngx_http_next_header_filter(r);
}
- } else if (r->gzip_ok) {
- return ngx_http_next_header_filter(r);
+ } else if (conf->enable == NGX_HTTP_GUNZIP_ALWAYS
+ && ngx_http_test_content_type(r, &conf->types) == NULL)
+ {
+ return ngx_http_next_header_filter(r);
}
ctx = ngx_pcalloc(r->pool, sizeof(ngx_http_gunzip_ctx_t));
@@ -651,9 +679,11 @@
* set by ngx_pcalloc():
*
* conf->bufs.num = 0;
+ * conf->types = { NULL };
+ * conf->types_keys = NULL;
*/
- conf->enable = NGX_CONF_UNSET;
+ conf->enable = NGX_CONF_UNSET_UINT;
return conf;
}
@@ -665,11 +695,20 @@
ngx_http_gunzip_conf_t *prev = parent;
ngx_http_gunzip_conf_t *conf = child;
- ngx_conf_merge_value(conf->enable, prev->enable, 0);
+ ngx_conf_merge_value(conf->enable, prev->enable,
+ NGX_HTTP_GUNZIP_OFF);
ngx_conf_merge_bufs_value(conf->bufs, prev->bufs,
(128 * 1024) / ngx_pagesize, ngx_pagesize);
+ if (ngx_http_merge_types(cf, &conf->types_keys, &conf->types,
+ &prev->types_keys, &prev->types,
+ ngx_http_html_default_types)
+ != NGX_OK)
+ {
+ return NGX_CONF_ERROR;
+ }
+
return NGX_CONF_OK;
}
_______________________________________________
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel
