Thanks! It looks good. Cheers, Alexey. вт, 26 окт. 2021 г., 03:42 Maxim Dounin <[email protected]>:
> Hello! > > On Thu, Aug 19, 2021 at 09:34:39PM +0300, Alexey Radkov wrote: > > > # HG changeset patch > > # User Alexey Radkov <[email protected]> > > # Date 1629395487 -10800 > > # Thu Aug 19 20:51:27 2021 +0300 > > # Node ID a1065b2252855730ed8e5368c88fe41a7ff5a698 > > # Parent 13d0c1d26d47c203b1874ca1ffdb7a9ba7fd2d77 > > Avoid unnecessary restriction on nohash http variables. > > > > When I use variables with long names albeit being tagged as > > NGX_HTTP_VARIABLE_NOHASH, Nginx says "could not build variables_hash, > > you should increase variables_hash_bucket_size: 64". It seems that this > is > > an unnecessary restriction, as soon as the hash gets only built for > variables > > with names[n].key.data == NULL (note that other pieces in ngx_hash_init() > > where the macro NGX_HASH_ELT_SIZE is used, are always guarded with this > > condition). This fix puts this same condition into the only unguarded > piece: > > when testing against the hash_bucket_size. > > > > The issue arises after assignment of key[n].key.data = NULL without > symmetric > > assignment of key[n].key.len in ngx_http_variables_init_vars(): after > this, > > the key[n].key comes to an inconsistent state. Perhaps this was made > > intentionally, as hash initialization in other places seems to follow the > > same pattern (for instance, see how ngx_hash_init() gets called from > > ngx_http_upstream_hide_headers_hash()). > > > > Without this fix, I must put in the config "variables_hash_bucket_size > 128;" > > even if the long-named variables are nohash. > > > > diff -r 13d0c1d26d47 -r a1065b225285 src/core/ngx_hash.c > > --- a/src/core/ngx_hash.c Fri Aug 13 03:57:47 2021 -0400 > > +++ b/src/core/ngx_hash.c Thu Aug 19 20:51:27 2021 +0300 > > @@ -274,6 +274,9 @@ > > } > > > > for (n = 0; n < nelts; n++) { > > + if (names[n].key.data == NULL) { > > + continue; > > + } > > if (hinit->bucket_size < NGX_HASH_ELT_SIZE(&names[n]) + > sizeof(void *)) > > { > > ngx_log_error(NGX_LOG_EMERG, hinit->pool->log, 0, > > > > Thanks for spotting this. > > Here is a version of the patch slightly cleaned up to better match > style, please take a look if looks good for you: > > # HG changeset patch > # User Alexey Radkov <[email protected]> > # Date 1629395487 -10800 > # Thu Aug 19 20:51:27 2021 +0300 > # Node ID 2a7155733855d1c2ea1c1ded8d1a4ba654b533cb > # Parent 3f0ab7b6cd71eb02b4714278cabcd2db5c79b3a9 > Core: removed unnecessary restriction in hash initialization. > > Hash initialization ignores elements with key.data set to NULL. > Nevertheless, the initial hash bucket size check didn't skip them, > resulting in unnecessary restrictions on, for example, variables with > long names and with the NGX_HTTP_VARIABLE_NOHASH flag. > > Fix is to update the initial hash bucket size check to skip elements > with key.data set to NULL, similarly to how it is done in other parts > of the code. > > diff --git a/src/core/ngx_hash.c b/src/core/ngx_hash.c > --- a/src/core/ngx_hash.c > +++ b/src/core/ngx_hash.c > @@ -274,6 +274,10 @@ ngx_hash_init(ngx_hash_init_t *hinit, ng > } > > for (n = 0; n < nelts; n++) { > + if (names[n].key.data == NULL) { > + continue; > + } > + > if (hinit->bucket_size < NGX_HASH_ELT_SIZE(&names[n]) + > sizeof(void *)) > { > ngx_log_error(NGX_LOG_EMERG, hinit->pool->log, 0, > > > -- > Maxim Dounin > http://mdounin.ru/ > _______________________________________________ > nginx-devel mailing list > [email protected] > http://mailman.nginx.org/mailman/listinfo/nginx-devel >
_______________________________________________ nginx-devel mailing list [email protected] http://mailman.nginx.org/mailman/listinfo/nginx-devel
