Mercurial > njs

changeset 2603:2db79dbf07f6

Find changesets by keywords (author, files, the commit message), revision number or hash, or revset expression.
Modules: fixed incorrect config rejections introduced in d157f56. d157f56 introduced configure time checks for js_set and js_periodic directives. It turned out to be too strict. The fix is to remove them completely as there is no way to track variable usage context in configure time.
author Dmitry Volyntsev <xeioex@nginx.com>
date Fri, 15 Aug 2025 17:28:47 -0700
parents 840a3f263d08
children 1f9b9265d79a
files nginx/ngx_http_js_module.c nginx/ngx_stream_js_module.c nginx/t/js_variables_location.t nginx/t/stream_js_variables_server.t
diffstat 4 files changed, 218 insertions(+), 106 deletions(-) [+]
line wrap: on
line diff
--- a/nginx/ngx_http_js_module.c	Mon Aug 11 23:26:12 2025 -0700
+++ b/nginx/ngx_http_js_module.c	Fri Aug 15 17:28:47 2025 -0700
@@ -40,9 +40,6 @@
     ngx_log_t              log;
     ngx_http_log_ctx_t     log_ctx;
     ngx_event_t            event;
-
-    u_char                *file_name;
-    ngx_uint_t             line;
 } ngx_js_periodic_t;
 
 
@@ -1540,6 +1537,9 @@
     }
 
     if (rc == NGX_DECLINED) {
+        ngx_log_error(NGX_LOG_ERR, r->connection->log, 0,
+                      "no \"js_import\" directives found for \"js_set\" handler"
+                      " \"%V\" in the current scope", fname);
         v->not_found = 1;
         return NGX_OK;
     }
@@ -4485,6 +4485,15 @@
 
     rc = ngx_http_js_init_vm(r, ngx_http_js_periodic_session_proto_id);
 
+    if (rc == NGX_DECLINED) {
+        ngx_log_error(NGX_LOG_ERR, r->connection->log, 0,
+                      "no \"js_import\" directives found for \"js_periodic\""
+                      " handler \"%V\" in the current scope",
+                      &periodic->method);
+        ngx_http_js_periodic_destroy(r, periodic);
+        return;
+    }
+
     if (rc != NGX_OK) {
         ngx_http_js_periodic_destroy(r, periodic);
         return;
@@ -7691,61 +7700,12 @@
 static ngx_int_t
 ngx_http_js_init(ngx_conf_t *cf)
 {
-    ngx_uint_t                  i, found_issue;
-    ngx_js_set_t               *data;
-    ngx_hash_key_t             *key;
-    ngx_http_variable_t        *v;
-    ngx_js_periodic_t          *periodic;
-    ngx_js_loc_conf_t          *jlcf;
-    ngx_js_main_conf_t         *jmcf;
-    ngx_http_core_main_conf_t  *cmcf;
-
     ngx_http_next_header_filter = ngx_http_top_header_filter;
     ngx_http_top_header_filter = ngx_http_js_header_filter;
 
     ngx_http_next_body_filter = ngx_http_top_body_filter;
     ngx_http_top_body_filter = ngx_http_js_body_filter;
 
-    jmcf = ngx_http_conf_get_module_main_conf(cf, ngx_http_js_module);
-    jlcf = ngx_http_conf_get_module_loc_conf(cf, ngx_http_js_module);
-
-    if (jlcf->engine == NULL) {
-        found_issue = 0;
-
-        if (jmcf->periodics != NULL) {
-            periodic = jmcf->periodics->elts;
-
-            for (i = 0; i < jmcf->periodics->nelts; i++) {
-                ngx_log_error(NGX_LOG_EMERG, cf->log, 0,
-                              "no \"js_import\" directives found for "
-                              "\"js_periodic\" \"%V\" in %s:%ui",
-                              &periodic[i].method, periodic[i].file_name,
-                              periodic[i].line);
-            }
-
-            found_issue = 1;
-        }
-
-        cmcf = ngx_http_conf_get_module_main_conf(cf, ngx_http_core_module);
-        key = cmcf->variables_keys->keys.elts;
-
-        for (i = 0; i < cmcf->variables_keys->keys.nelts; i++) {
-            v = key[i].value;
-            if (v->get_handler == ngx_http_js_variable_set) {
-                data = (ngx_js_set_t *) v->data;
-                ngx_log_error(NGX_LOG_EMERG, cf->log, 0,
-                              "no \"js_import\" directives found for "
-                              "\"js_set\" \"$%V\" \"%V\" in %s:%ui", &v->name,
-                              &data->fname, data->file_name, data->line);
-                found_issue = 1;
-            }
-        }
-
-        if (found_issue) {
-            return NGX_ERROR;
-        }
-    }
-
     return NGX_OK;
 }
 
@@ -7944,8 +7904,6 @@
     periodic->jitter = jitter;
     periodic->worker_affinity = mask;
     periodic->conf_ctx = cf->ctx;
-    periodic->file_name = cf->conf_file->file.name.data;
-    periodic->line = cf->conf_file->line;
 
     return NGX_CONF_OK;
 }
--- a/nginx/ngx_stream_js_module.c	Mon Aug 11 23:26:12 2025 -0700
+++ b/nginx/ngx_stream_js_module.c	Fri Aug 15 17:28:47 2025 -0700
@@ -46,9 +46,6 @@
 
     ngx_log_t               log;
     ngx_event_t             event;
-
-    u_char                 *file_name;
-    ngx_uint_t              line;
 } ngx_js_periodic_t;
 
 
@@ -1068,6 +1065,9 @@
     }
 
     if (rc == NGX_DECLINED) {
+        ngx_log_error(NGX_LOG_ERR, s->connection->log, 0,
+                      "no \"js_import\" directives found for \"js_set\" handler"
+                      " \"%V\" in the current scope", fname);
         v->not_found = 1;
         return NGX_OK;
     }
@@ -3061,6 +3061,15 @@
 
     rc = ngx_stream_js_init_vm(s, ngx_stream_js_periodic_session_proto_id);
 
+    if (rc == NGX_DECLINED) {
+        ngx_log_error(NGX_LOG_ERR, &periodic->log, 0,
+                      "no \"js_import\" directives found for \"js_periodic\""
+                      " handler \"%V\" in the current scope",
+                      &periodic->method);
+        ngx_stream_js_periodic_destroy(s, periodic);
+        return;
+    }
+
     if (rc != NGX_OK) {
         ngx_stream_js_periodic_destroy(s, periodic);
         return;
@@ -3395,8 +3404,6 @@
     periodic->jitter = jitter;
     periodic->worker_affinity = mask;
     periodic->conf_ctx = cf->ctx;
-    periodic->file_name = cf->conf_file->file.name.data;
-    periodic->line = cf->conf_file->line;
 
     return NGX_CONF_OK;
 }
@@ -3617,14 +3624,6 @@
 static ngx_int_t
 ngx_stream_js_init(ngx_conf_t *cf)
 {
-    ngx_uint_t                   i;
-    ngx_flag_t                   found_issue;
-    ngx_js_set_t                *data;
-    ngx_hash_key_t              *key;
-    ngx_stream_variable_t       *v;
-    ngx_js_periodic_t           *periodic;
-    ngx_js_loc_conf_t           *jlcf;
-    ngx_js_main_conf_t          *jmcf;
     ngx_stream_handler_pt       *h;
     ngx_stream_core_main_conf_t *cmcf;
 
@@ -3647,45 +3646,6 @@
 
     *h = ngx_stream_js_preread_handler;
 
-    jmcf = ngx_stream_conf_get_module_main_conf(cf, ngx_stream_js_module);
-    jlcf = ngx_stream_conf_get_module_srv_conf(cf, ngx_stream_js_module);
-
-    if (jlcf->engine == NULL) {
-        found_issue = 0;
-
-        if (jmcf->periodics != NULL) {
-            periodic = jmcf->periodics->elts;
-
-            for (i = 0; i < jmcf->periodics->nelts; i++) {
-                ngx_log_error(NGX_LOG_EMERG, cf->log, 0,
-                              "no \"js_import\" directives found for "
-                              "\"js_periodic\" \"%V\" in %s:%ui",
-                              &periodic[i].method, periodic[i].file_name,
-                              periodic[i].line);
-            }
-
-            found_issue = 1;
-        }
-
-        key = cmcf->variables_keys->keys.elts;
-
-        for (i = 0; i < cmcf->variables_keys->keys.nelts; i++) {
-            v = key[i].value;
-            if (v->get_handler == ngx_stream_js_variable_set) {
-                data = (ngx_js_set_t *) v->data;
-                ngx_log_error(NGX_LOG_EMERG, cf->log, 0,
-                              "no \"js_import\" directives found for "
-                              "\"js_set\" \"$%V\" \"%V\" in %s:%ui", &v->name,
-                              &data->fname, data->file_name, data->line);
-                found_issue = 1;
-            }
-        }
-
-        if (found_issue) {
-            return NGX_ERROR;
-        }
-    }
-
     return NGX_OK;
 }
 
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/nginx/t/js_variables_location.t	Fri Aug 15 17:28:47 2025 -0700
@@ -0,0 +1,96 @@
+#!/usr/bin/perl
+
+# (C) Dmitry Volyntsev
+# (C) Nginx, Inc.
+
+# Tests for http njs module, setting nginx variables, location js_import.
+
+###############################################################################
+
+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 rewrite/)
+	->write_file_expand('nginx.conf', <<'EOF');
+
+%%TEST_GLOBALS%%
+
+daemon off;
+
+events {
+}
+
+http {
+    %%TEST_GLOBALS_HTTP%%
+
+    server {
+        listen       127.0.0.1:8080;
+        server_name  localhost;
+
+        location /foo {
+            js_import main from foo.js;
+            js_set $test_var   main.variable;
+
+            return 200 $test_var;
+        }
+
+        location /bar {
+            js_import main from bar.js;
+            js_set $test_var   main.variable;
+
+            return 200 $test_var;
+        }
+
+        location /not_found {
+            return 200 "NOT_FOUND:$test_var";
+        }
+    }
+}
+
+EOF
+
+$t->write_file('foo.js', <<EOF);
+    function variable(r) {
+        return 'foo_var';
+    }
+
+    export default {variable};
+
+EOF
+
+$t->write_file('bar.js', <<EOF);
+    function variable(r) {
+        return 'bar_var';
+    }
+
+    export default {variable};
+
+EOF
+
+$t->try_run('no njs')->plan(4);
+
+###############################################################################
+
+like(http_get('/foo'), qr/foo_var/, 'foo var');
+like(http_get('/bar'), qr/bar_var/, 'bar var');
+like(http_get('/not_found'), qr/NOT_FOUND:$/, 'not found is empty');
+
+$t->stop();
+
+ok(index($t->read_file('error.log'),
+	'no "js_import" directives found for "js_set" handler "main.variable" '
+	. 'in the current scope') > 0, 'log error for js_set without js_import');
+
+###############################################################################
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/nginx/t/stream_js_variables_server.t	Fri Aug 15 17:28:47 2025 -0700
@@ -0,0 +1,98 @@
+#!/usr/bin/perl
+
+# (C) Dmitry Volyntsev
+# (C) Nginx, Inc.
+
+# Tests for stream njs module, setting nginx variables, server js_import.
+
+###############################################################################
+
+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%%
+
+    server {
+        listen  127.0.0.1:8081;
+
+        js_import main from foo.js;
+        js_set $test_var main.variable;
+
+        return  $test_var;
+    }
+
+    server {
+        listen  127.0.0.1:8082;
+
+        js_import main from bar.js;
+        js_set $test_var main.variable;
+
+        return  $test_var;
+    }
+
+    server {
+        listen  127.0.0.1:8083;
+
+        return  "NOT_FOUND:$test_var";
+    }
+}
+
+EOF
+
+$t->write_file('foo.js', <<EOF);
+    function variable(s) {
+        return 'foo_var';
+    }
+
+    export default {variable};
+
+EOF
+
+$t->write_file('bar.js', <<EOF);
+    function variable(s) {
+        return 'bar_var';
+    }
+
+    export default {variable};
+
+EOF
+
+$t->try_run('no stream njs available')->plan(4);
+
+###############################################################################
+
+is(stream('127.0.0.1:' . port(8081))->read(), 'foo_var', 'foo var');
+is(stream('127.0.0.1:' . port(8082))->read(), 'bar_var', 'bar var');
+is(stream('127.0.0.1:' . port(8083))->read(), 'NOT_FOUND:', 'not found var');
+
+$t->stop();
+
+ok(index($t->read_file('error.log'),
+	'no "js_import" directives found for "js_set" handler "main.variable" '
+	. 'in the current scope') > 0, 'log error for js_set without js_import');
+
+###############################################################################