details:
https://github.com/nginx/njs/commit/2c93ca079f497b68ea1680d282f705c926cf7210
branches: master
commit: 2c93ca079f497b68ea1680d282f705c926cf7210
user: Dmitry Volyntsev <xei...@nginx.com>
date: Fri, 15 Aug 2025 17:28:47 -0700
description:
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.
---
nginx/ngx_http_js_module.c | 66 +++++-------------------
nginx/ngx_stream_js_module.c | 64 +++++------------------
nginx/t/js_variables_location.t | 96 +++++++++++++++++++++++++++++++++++
nginx/t/stream_js_variables_server.t | 98 ++++++++++++++++++++++++++++++++++++
4 files changed, 218 insertions(+), 106 deletions(-)
diff --git a/nginx/ngx_http_js_module.c b/nginx/ngx_http_js_module.c
index 578aec42..402c7382 100644
--- a/nginx/ngx_http_js_module.c
+++ b/nginx/ngx_http_js_module.c
@@ -40,9 +40,6 @@ typedef struct {
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 @@ ngx_http_js_variable_set(ngx_http_request_t *r,
ngx_http_variable_value_t *v,
}
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 @@ ngx_http_js_periodic_handler(ngx_event_t *ev)
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 @@ ngx_http_js_init_conf_vm(ngx_conf_t *cf,
ngx_js_loc_conf_t *conf)
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 @@ invalid:
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;
}
diff --git a/nginx/ngx_stream_js_module.c b/nginx/ngx_stream_js_module.c
index 396a71e2..a3a85662 100644
--- a/nginx/ngx_stream_js_module.c
+++ b/nginx/ngx_stream_js_module.c
@@ -46,9 +46,6 @@ typedef struct {
ngx_log_t log;
ngx_event_t event;
-
- u_char *file_name;
- ngx_uint_t line;
} ngx_js_periodic_t;
@@ -1068,6 +1065,9 @@ ngx_stream_js_variable_set(ngx_stream_session_t *s,
}
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 @@ ngx_stream_js_periodic_handler(ngx_event_t *ev)
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 @@ invalid:
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 @@ ngx_stream_js_shared_dict_zone(ngx_conf_t *cf,
ngx_command_t *cmd,
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 @@ ngx_stream_js_init(ngx_conf_t *cf)
*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;
}
diff --git a/nginx/t/js_variables_location.t b/nginx/t/js_variables_location.t
new file mode 100644
index 00000000..02bd85ef
--- /dev/null
+++ b/nginx/t/js_variables_location.t
@@ -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');
+
+###############################################################################
diff --git a/nginx/t/stream_js_variables_server.t
b/nginx/t/stream_js_variables_server.t
new file mode 100644
index 00000000..a7d53718
--- /dev/null
+++ b/nginx/t/stream_js_variables_server.t
@@ -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');
+
+###############################################################################
_______________________________________________
nginx-devel mailing list
nginx-devel@nginx.org
https://mailman.nginx.org/mailman/listinfo/nginx-devel
