[njs] Modules: improved checking for duplicate js_set variables.

Dmitry Volyntsev xeioex at nginx.com
Tue Apr 23 01:09:19 UTC 2024


details:   https://hg.nginx.org/njs/rev/be271e8d0b3b
branches:  
changeset: 2318:be271e8d0b3b
user:      Dmitry Volyntsev <xeioex at nginx.com>
date:      Mon Apr 22 17:51:45 2024 -0700
description:
Modules: improved checking for duplicate js_set variables.

Since 6fb1aca4eeaf (0.8.4) the identical js_set variables introduced as
a part of an include file that is shared amongst multiple vhosts are
rejected during configuration parsing.

The patch ignores duplicate js_set variables when they refer to the same
JS function.

This fixes #705 issue on Github.

diffstat:

 nginx/ngx_http_js_module.c   |  15 ++++++--
 nginx/ngx_stream_js_module.c |  15 ++++++--
 nginx/t/js_dup_set.t         |  74 ++++++++++++++++++++++++++++++++++++++++++++
 nginx/t/stream_js_dup_set.t  |  72 ++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 168 insertions(+), 8 deletions(-)

diffs (218 lines):

diff -r b73f99c7bc54 -r be271e8d0b3b nginx/ngx_http_js_module.c
--- a/nginx/ngx_http_js_module.c	Mon Apr 22 17:51:24 2024 -0700
+++ b/nginx/ngx_http_js_module.c	Mon Apr 22 17:51:45 2024 -0700
@@ -4732,7 +4732,7 @@ invalid:
 static char *
 ngx_http_js_set(ngx_conf_t *cf, ngx_command_t *cmd, void *conf)
 {
-    ngx_str_t            *value, *fname;
+    ngx_str_t            *value, *fname, *prev;
     ngx_http_variable_t  *v;
 
     value = cf->args->elts;
@@ -4759,9 +4759,16 @@ ngx_http_js_set(ngx_conf_t *cf, ngx_comm
     *fname = value[2];
 
     if (v->get_handler == ngx_http_js_variable_set) {
-        ngx_conf_log_error(NGX_LOG_EMERG, cf, 0,
-                           "variable \"%V\" is already declared", &value[1]);
-        return NGX_CONF_ERROR;
+        prev = (ngx_str_t *) v->data;
+
+        if (fname->len != prev->len
+            || ngx_strncmp(fname->data, prev->data, fname->len) != 0)
+        {
+            ngx_conf_log_error(NGX_LOG_EMERG, cf, 0,
+                               "variable \"%V\" is redeclared with "
+                               "different function name", &value[1]);
+            return NGX_CONF_ERROR;
+        }
     }
 
     v->get_handler = ngx_http_js_variable_set;
diff -r b73f99c7bc54 -r be271e8d0b3b nginx/ngx_stream_js_module.c
--- a/nginx/ngx_stream_js_module.c	Mon Apr 22 17:51:24 2024 -0700
+++ b/nginx/ngx_stream_js_module.c	Mon Apr 22 17:51:45 2024 -0700
@@ -2191,7 +2191,7 @@ invalid:
 static char *
 ngx_stream_js_set(ngx_conf_t *cf, ngx_command_t *cmd, void *conf)
 {
-    ngx_str_t              *value, *fname;
+    ngx_str_t              *value, *fname, *prev;
     ngx_stream_variable_t  *v;
 
     value = cf->args->elts;
@@ -2218,9 +2218,16 @@ ngx_stream_js_set(ngx_conf_t *cf, ngx_co
     *fname = value[2];
 
     if (v->get_handler == ngx_stream_js_variable_set) {
-        ngx_conf_log_error(NGX_LOG_EMERG, cf, 0,
-                           "variable \"%V\" is already declared", &value[1]);
-        return NGX_CONF_ERROR;
+        prev = (ngx_str_t *) v->data;
+
+        if (fname->len != prev->len
+            || ngx_strncmp(fname->data, prev->data, fname->len) != 0)
+        {
+            ngx_conf_log_error(NGX_LOG_EMERG, cf, 0,
+                               "variable \"%V\" is redeclared with "
+                               "different function name", &value[1]);
+            return NGX_CONF_ERROR;
+        }
     }
 
     v->get_handler = ngx_stream_js_variable_set;
diff -r b73f99c7bc54 -r be271e8d0b3b nginx/t/js_dup_set.t
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/nginx/t/js_dup_set.t	Mon Apr 22 17:51:45 2024 -0700
@@ -0,0 +1,74 @@
+#!/usr/bin/perl
+
+# (C) Dmitry Volyntsev
+# (C) Nginx, Inc.
+
+# Tests for http njs module, duplicate identical js_set directives.
+
+###############################################################################
+
+use warnings;
+use strict;
+
+use Test::More;
+
+BEGIN { use FindBin; chdir($FindBin::Bin); }
+
+use lib 'lib';
+use Test::Nginx;
+
+###############################################################################
+
+select STDERR; $| = 1;
+select STDOUT; $| = 1;
+
+my $t = Test::Nginx->new()->has(qw/http/)
+	->write_file_expand('nginx.conf', <<'EOF');
+
+%%TEST_GLOBALS%%
+
+daemon off;
+
+events {
+}
+
+http {
+    %%TEST_GLOBALS_HTTP%%
+
+    js_import test.js;
+
+    server {
+        listen       127.0.0.1:8080;
+        server_name  localhost;
+
+        location /set1 {
+            js_set $test test.foo;
+            return 200 set1:$test;
+        }
+
+        location /set2 {
+            js_set $test test.foo;
+            return 200 set2:$test;
+        }
+    }
+}
+
+EOF
+
+$t->write_file('test.js', <<EOF);
+    function foo(r) {
+        return 42;
+    }
+
+    export default {foo};
+
+EOF
+
+$t->try_run('no njs')->plan(2);
+
+###############################################################################
+
+like(http_get('/set1'), qr/set1:42/, '/set1 location');
+like(http_get('/set2'), qr/set2:42/, '/set2 location');
+
+###############################################################################
diff -r b73f99c7bc54 -r be271e8d0b3b nginx/t/stream_js_dup_set.t
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/nginx/t/stream_js_dup_set.t	Mon Apr 22 17:51:45 2024 -0700
@@ -0,0 +1,72 @@
+#!/usr/bin/perl
+
+# (C) Dmitry Volyntsev
+# (C) Nginx, Inc.
+
+# Tests for stream njs module, duplicate identical js_set directives.
+
+###############################################################################
+
+use warnings;
+use strict;
+
+use Test::More;
+
+BEGIN { use FindBin; chdir($FindBin::Bin); }
+
+use lib 'lib';
+use Test::Nginx;
+use Test::Nginx::Stream qw/ stream /;
+
+###############################################################################
+
+select STDERR; $| = 1;
+select STDOUT; $| = 1;
+
+my $t = Test::Nginx->new()->has(qw/stream stream_return/)
+	->write_file_expand('nginx.conf', <<'EOF');
+
+%%TEST_GLOBALS%%
+
+daemon off;
+
+events {
+}
+
+stream {
+    %%TEST_GLOBALS_STREAM%%
+
+    js_import test.js;
+
+    server {
+        listen  127.0.0.1:8081;
+        js_set $test test.foo;
+        return  8081:$test;
+    }
+
+    server {
+        listen  127.0.0.1:8082;
+        js_set $test test.foo1;
+        return  8082:$test;
+    }
+}
+
+EOF
+
+$t->write_file('test.js', <<EOF);
+    function foo(r) {
+        return 42;
+    }
+
+    export default {foo};
+
+EOF
+
+$t->try_run('no njs available')->plan(2);
+
+###############################################################################
+
+is(stream('127.0.0.1:' . port(8081))->read(), '8081:42', '8081 server');
+is(stream('127.0.0.1:' . port(8082))->read(), '8082:42', '8082 server');
+
+###############################################################################


More information about the nginx-devel mailing list