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