Thank you very much!

this is my new patch, I use bitmask and ngx_list.

Index: src/http/modules/ngx_http_fastcgi_module.c
===================================================================
--- src/http/modules/ngx_http_fastcgi_module.c (revision 3937)
+++ src/http/modules/ngx_http_fastcgi_module.c (working copy)
@@ -691,11 +691,13 @@
u_char ch, *pos, *lowcase_key;
size_t size, len, key_len, val_len, padding,
allocated;
- ngx_uint_t i, n, next, hash, header_params;
+ uint64_t *bitmask;
+ ngx_uint_t i, k, n, next, hash, bm_index;
ngx_buf_t *b;
+ ngx_list_t ignore_bitmask;
ngx_chain_t *cl, *body;
- ngx_list_part_t *part;
- ngx_table_elt_t *header, **ignored;
+ ngx_list_part_t *part, *bitmask_part;
+ ngx_table_elt_t *header;
ngx_http_script_code_pt code;
ngx_http_script_engine_t e, le;
ngx_http_fastcgi_header_t *h;
@@ -703,8 +705,8 @@
ngx_http_script_len_code_pt lcode;

len = 0;
- header_params = 0;
- ignored = NULL;
+ bm_index = 0;
+ bitmask = NULL;

flcf = ngx_http_get_module_loc_conf(r, ngx_http_fastcgi_module);

@@ -737,16 +739,24 @@
lowcase_key = NULL;

if (flcf->header_params) {
- ignored = ngx_palloc(r->pool, flcf->header_params * sizeof(void
*));
- if (ignored == NULL) {
+ if (ngx_list_init(&ignore_bitmask, r->pool, 3,
sizeof(uint64_t))
+ != NGX_OK)
+ {
return NGX_ERROR;
}
+
+ bitmask = ngx_list_push(&ignore_bitmask);
+ if (bitmask == NULL) {
+ return NGX_ERROR;
+ }
+
+ *bitmask = 0;
}

part = &r->headers_in.headers.part;
header = part->elts;

- for (i = 0; /* void */; i++) {
+ for (i = 0, k = 0; /* void */; i++, k++) {

if (i >= part->nelts) {
if (part->next == NULL) {
@@ -767,6 +777,16 @@
}
}

+ if (k >= sizeof(uint64_t) * 8) {
+ bitmask = ngx_list_push(&ignore_bitmask);
+ if (bitmask == NULL) {
+ return NGX_ERROR;
+ }
+
+ *bitmask = 0;
+ k = 0;
+ }
+
hash = 0;

for (n = 0; n < header[i].key.len; n++) {
@@ -784,7 +804,7 @@
}

if (ngx_hash_find(&flcf->headers_hash, hash, lowcase_key,
n)) {
- ignored[header_params++] = &header[i];
+ *bitmask |= (uint64_t) 1 << k;
continue;
}

@@ -903,8 +923,13 @@
part = &r->headers_in.headers.part;
header = part->elts;

- for (i = 0; /* void */; i++) {
+ if (flcf->header_params) {
+ bitmask_part = &ignore_bitmask.part;
+ bitmask = bitmask_part->elts;
+ }

+ for (i = 0, k = 0; /* void */; i++, k++) {
+
if (i >= part->nelts) {
if (part->next == NULL) {
break;
@@ -915,10 +940,26 @@
i = 0;
}

- for (n = 0; n < header_params; n++) {
- if (&header[i] == ignored[n]) {
- goto next;
+ if (flcf->header_params) {
+
+ if (k >= sizeof(uint64_t) * 8) {
+ k = 0;
+ bm_index++;
}
+
+ if (bm_index >= bitmask_part->nelts) {
+ if (bitmask_part->next == NULL) {
+ break;
+ }
+
+ bitmask_part = bitmask_part->next;
+ bitmask = bitmask_part->elts;
+ bm_index = 0;
+ }
+
+ if (bitmask[bm_index] & ((uint64_t) 1 << k)) {
+ continue;
+ }
}

key_len = sizeof("HTTP_") - 1 + header[i].key.len;
@@ -964,9 +1005,6 @@
"fastcgi param: \"%*s: %*s\"",
key_len, b->last - (key_len + val_len),
val_len, b->last - val_len);
- next:
-
- continue;
}
}




On Sat, Jun 11, 2011 at 8:30 PM, Maxim Dounin <mdounin@mdounin.ru> wrote:

> Hello!
>
> On Fri, Jun 10, 2011 at 04:55:55PM +0800, Simon Liu wrote:
>
> > Thanks!
> >
> > I refactor the prev patch.
> >
> > this is new patch:
>
> [...]
>
> > @@ -707,7 +769,10 @@
> > ignored = NULL;
> >
> > flcf = ngx_http_get_module_loc_conf(r, ngx_http_fastcgi_module);
> > + cscf = ngx_http_get_module_srv_conf(r, ngx_http_core_module);
> >
> > + allow_underscores = cscf->underscores_in_headers;
> > +
>
> This is a layering violation. This is also a good example of why
> layering violation is bad: this is incorrect as underscores may as
> well appear in headers with "ignore_invalid_headers off;", even if
> undescores_in_headers is off.
>
> > if (flcf->params_len) {
> > ngx_memzero(&le, sizeof(ngx_http_script_engine_t));
> >
> > @@ -784,6 +849,14 @@
> > }
> >
> > if (ngx_hash_find(&flcf->headers_hash, hash,
> lowcase_key,
> > n)) {
> > +
> > + if (header_params == flcf->header_params
> > + || ngx_http_fastcgi_ignored_header(ignored,
> > &header[i],
> > + header_params, allow_underscores) == NGX_OK)
> > + {
> > + continue;
> > + }
> > +
> > ignored[header_params++] = &header[i];
>
> I would suggest using bitmask sized after number of headers in
> request instead.
>
> Alternatively, nginx dynamically sized array or list may be used
> for ignored, this should be a bit closer to original code. Though
> I personally think bitmask would be better.
>
> Maxim Dounin
>
> _______________________________________________
> nginx-devel mailing list
> nginx-devel@nginx.org
> http://nginx.org/mailman/listinfo/nginx-devel
>



--
博观约取

豆瓣:www.douban.com/people/mustang/

blog: www.pagefault.info

twitter: www.twitter.com/minibobo

sina 微博: www.weibo.com/diaoliang
_______________________________________________
nginx-devel mailing list
nginx-devel@nginx.org
http://nginx.org/mailman/listinfo/nginx-devel