[njs] Modules: added nocache flag for js_set variables.

noreply at nginx.com noreply at nginx.com
Sat Aug 24 00:34:02 UTC 2024


details:   https://github.com/nginx/njs/commit/5b7905c57fcb994a880804066c6abec32490180c
branches:  master
commit:    5b7905c57fcb994a880804066c6abec32490180c
user:      Thomas P. <TPXP at users.noreply.github.com>
date:      Wed, 7 Aug 2024 11:47:08 +0200
description:
Modules: added nocache flag for js_set variables.

This commit adds support for an additional `nocache` flag in `js_set`
directives. If set, the resulting nginx variable will have no_cacheable set
to 1. This enables us to dynamically recompute a variable if the context
changed (for example, in case of an internal redirection).

In case of multiple calls in a location, users should cache the result in a
rewrite variable: `set $cached_variable $js_variable;`

---
 nginx/ngx_http_js_module.c            | 38 ++++++++++----
 nginx/ngx_js.h                        |  8 +++
 nginx/ngx_stream_js_module.c          | 37 +++++++++----
 nginx/t/js_variables_nocache.t        | 99 +++++++++++++++++++++++++++++++++++
 nginx/t/stream_js_variables_nocache.t | 94 +++++++++++++++++++++++++++++++++
 5 files changed, 254 insertions(+), 22 deletions(-)

diff --git a/nginx/ngx_http_js_module.c b/nginx/ngx_http_js_module.c
index 824b647c..7f2eded2 100644
--- a/nginx/ngx_http_js_module.c
+++ b/nginx/ngx_http_js_module.c
@@ -342,7 +342,7 @@ static ngx_command_t  ngx_http_js_commands[] = {
       NULL },
 
     { ngx_string("js_set"),
-      NGX_HTTP_MAIN_CONF|NGX_HTTP_SRV_CONF|NGX_HTTP_LOC_CONF|NGX_CONF_TAKE2,
+      NGX_HTTP_MAIN_CONF|NGX_HTTP_SRV_CONF|NGX_HTTP_LOC_CONF|NGX_CONF_TAKE23,
       ngx_http_js_set,
       0,
       0,
@@ -1245,13 +1245,16 @@ static ngx_int_t
 ngx_http_js_variable_set(ngx_http_request_t *r, ngx_http_variable_value_t *v,
     uintptr_t data)
 {
-    ngx_str_t *fname = (ngx_str_t *) data;
+    ngx_js_set_t *vdata = (ngx_js_set_t *) data;
 
     ngx_int_t           rc;
     njs_int_t           pending;
+    ngx_str_t          *fname;
     njs_str_t           value;
     ngx_http_js_ctx_t  *ctx;
 
+    fname = &vdata->fname;
+
     rc = ngx_http_js_init_vm(r, ngx_http_js_request_proto_id);
 
     if (rc == NGX_ERROR) {
@@ -1290,7 +1293,7 @@ ngx_http_js_variable_set(ngx_http_request_t *r, ngx_http_variable_value_t *v,
 
     v->len = value.length;
     v->valid = 1;
-    v->no_cacheable = 0;
+    v->no_cacheable = vdata->flags & NGX_NJS_VAR_NOCACHE;
     v->not_found = 0;
     v->data = value.start;
 
@@ -4639,7 +4642,8 @@ invalid:
 static char *
 ngx_http_js_set(ngx_conf_t *cf, ngx_command_t *cmd, void *conf)
 {
-    ngx_str_t            *value, *fname, *prev;
+    ngx_str_t            *value;
+    ngx_js_set_t         *data, *prev;
     ngx_http_variable_t  *v;
 
     value = cf->args->elts;
@@ -4658,18 +4662,19 @@ ngx_http_js_set(ngx_conf_t *cf, ngx_command_t *cmd, void *conf)
         return NGX_CONF_ERROR;
     }
 
-    fname = ngx_palloc(cf->pool, sizeof(ngx_str_t));
-    if (fname == NULL) {
+    data = ngx_palloc(cf->pool, sizeof(ngx_js_set_t));
+    if (data == NULL) {
         return NGX_CONF_ERROR;
     }
 
-    *fname = value[2];
+    data->fname = value[2];
+    data->flags = 0;
 
     if (v->get_handler == ngx_http_js_variable_set) {
-        prev = (ngx_str_t *) v->data;
+        prev = (ngx_js_set_t *) v->data;
 
-        if (fname->len != prev->len
-            || ngx_strncmp(fname->data, prev->data, fname->len) != 0)
+        if (data->fname.len != prev->fname.len
+            || ngx_strncmp(data->fname.data, prev->fname.data, data->fname.len) != 0)
         {
             ngx_conf_log_error(NGX_LOG_EMERG, cf, 0,
                                "variable \"%V\" is redeclared with "
@@ -4678,8 +4683,19 @@ ngx_http_js_set(ngx_conf_t *cf, ngx_command_t *cmd, void *conf)
         }
     }
 
+    if (cf->args->nelts == 4) {
+        if (ngx_strcmp(value[3].data, "nocache") == 0) {
+            data->flags |= NGX_NJS_VAR_NOCACHE;
+
+        } else {
+            ngx_conf_log_error(NGX_LOG_EMERG, cf, 0,
+                "unrecognized flag \"%V\"", &value[3]);
+            return NGX_CONF_ERROR;
+        }
+    }
+
     v->get_handler = ngx_http_js_variable_set;
-    v->data = (uintptr_t) fname;
+    v->data = (uintptr_t) data;
 
     return NGX_CONF_OK;
 }
diff --git a/nginx/ngx_js.h b/nginx/ngx_js.h
index aad88b54..373dd499 100644
--- a/nginx/ngx_js.h
+++ b/nginx/ngx_js.h
@@ -31,6 +31,8 @@
 #define NGX_JS_BOOL_TRUE    1
 #define NGX_JS_BOOL_UNSET   2
 
+#define NGX_NJS_VAR_NOCACHE 1
+
 #define ngx_js_buffer_type(btype) ((btype) & ~NGX_JS_DEPRECATED)
 
 
@@ -137,6 +139,12 @@ typedef struct {
 } ngx_js_loc_conf_t;
 
 
+typedef struct {
+    ngx_str_t fname;
+    unsigned  flags;
+} ngx_js_set_t;
+
+
 struct ngx_js_ctx_s {
     NGX_JS_COMMON_CTX;
 };
diff --git a/nginx/ngx_stream_js_module.c b/nginx/ngx_stream_js_module.c
index 4de0cb4f..295b2fcf 100644
--- a/nginx/ngx_stream_js_module.c
+++ b/nginx/ngx_stream_js_module.c
@@ -199,7 +199,7 @@ static ngx_command_t  ngx_stream_js_commands[] = {
       NULL },
 
     { ngx_string("js_set"),
-      NGX_STREAM_MAIN_CONF|NGX_STREAM_SRV_CONF|NGX_CONF_TAKE2,
+      NGX_STREAM_MAIN_CONF|NGX_STREAM_SRV_CONF|NGX_CONF_TAKE23,
       ngx_stream_js_set,
       0,
       0,
@@ -891,13 +891,16 @@ static ngx_int_t
 ngx_stream_js_variable_set(ngx_stream_session_t *s,
     ngx_stream_variable_value_t *v, uintptr_t data)
 {
-    ngx_str_t *fname = (ngx_str_t *) data;
+    ngx_js_set_t *vdata = (ngx_js_set_t *) data;
 
     ngx_int_t             rc;
     njs_int_t             pending;
+    ngx_str_t            *fname;
     njs_str_t             value;
     ngx_stream_js_ctx_t  *ctx;
 
+    fname = &vdata->fname;
+
     rc = ngx_stream_js_init_vm(s, ngx_stream_js_session_proto_id);
 
     if (rc == NGX_ERROR) {
@@ -936,7 +939,7 @@ ngx_stream_js_variable_set(ngx_stream_session_t *s,
 
     v->len = value.length;
     v->valid = 1;
-    v->no_cacheable = 0;
+    v->no_cacheable = vdata->flags & NGX_NJS_VAR_NOCACHE;
     v->not_found = 0;
     v->data = value.start;
 
@@ -2186,7 +2189,8 @@ invalid:
 static char *
 ngx_stream_js_set(ngx_conf_t *cf, ngx_command_t *cmd, void *conf)
 {
-    ngx_str_t              *value, *fname, *prev;
+    ngx_str_t              *value;
+    ngx_js_set_t           *data, *prev;
     ngx_stream_variable_t  *v;
 
     value = cf->args->elts;
@@ -2205,18 +2209,18 @@ ngx_stream_js_set(ngx_conf_t *cf, ngx_command_t *cmd, void *conf)
         return NGX_CONF_ERROR;
     }
 
-    fname = ngx_palloc(cf->pool, sizeof(ngx_str_t));
-    if (fname == NULL) {
+    data = ngx_palloc(cf->pool, sizeof(ngx_js_set_t));
+    if (data == NULL) {
         return NGX_CONF_ERROR;
     }
 
-    *fname = value[2];
+    data->fname = value[2];
 
     if (v->get_handler == ngx_stream_js_variable_set) {
-        prev = (ngx_str_t *) v->data;
+        prev = (ngx_js_set_t *) v->data;
 
-        if (fname->len != prev->len
-            || ngx_strncmp(fname->data, prev->data, fname->len) != 0)
+        if (data->fname.len != prev->fname.len
+            || ngx_strncmp(data->fname.data, prev->fname.data, data->fname.len) != 0)
         {
             ngx_conf_log_error(NGX_LOG_EMERG, cf, 0,
                                "variable \"%V\" is redeclared with "
@@ -2225,8 +2229,19 @@ ngx_stream_js_set(ngx_conf_t *cf, ngx_command_t *cmd, void *conf)
         }
     }
 
+    if (cf->args->nelts == 4) {
+        if (ngx_strcmp(value[3].data, "nocache") == 0) {
+            data->flags |= NGX_NJS_VAR_NOCACHE;
+
+        } else {
+            ngx_conf_log_error(NGX_LOG_EMERG, cf, 0,
+                "unrecognized flag \"%V\"", &value[3]);
+            return NGX_CONF_ERROR;
+        }
+    }
+
     v->get_handler = ngx_stream_js_variable_set;
-    v->data = (uintptr_t) fname;
+    v->data = (uintptr_t) data;
 
     return NGX_CONF_OK;
 }
diff --git a/nginx/t/js_variables_nocache.t b/nginx/t/js_variables_nocache.t
new file mode 100644
index 00000000..abf2b96d
--- /dev/null
+++ b/nginx/t/js_variables_nocache.t
@@ -0,0 +1,99 @@
+#!/usr/bin/perl
+
+# (C) Thomas P.
+
+# Tests for http njs module, setting non-cacheable nginx variables.
+
+###############################################################################
+
+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%%
+
+    js_set $nocache_var   test.variable nocache;
+    js_set $default_var   test.variable;
+    js_set $callcount_var test.callCount nocache;
+
+    js_import test.js;
+
+    server {
+        listen       127.0.0.1:8080;
+        server_name  localhost;
+
+        location /default_var {
+            set $a $default_var;
+            set $b $default_var;
+            return 200 '"$a/$b"';
+        }
+
+        location /nocache_var {
+            set $a $nocache_var;
+            set $b $nocache_var;
+            return 200 '"$a/$b"';
+        }
+
+        location /callcount_var {
+            set $a $callcount_var;
+            set $b $callcount_var;
+            return 200 '"$a/$b"';
+        }
+    }
+}
+
+EOF
+
+$t->write_file('test.js', <<EOF);
+    function variable(r) {
+        return Math.random().toFixed(16);
+    }
+
+    let n = 0;
+    function callCount(r) {
+        return (n += 1).toString();
+    }
+
+    export default {variable, callCount};
+
+EOF
+
+$t->try_run('no nocache njs variables')->plan(3);
+
+###############################################################################
+
+# We use backreferences to make sure the same value was returned for the two uses
+like(http_get('/default_var'), qr/"(.+)\/\1"/, 'cached variable');
+# Negative lookaheads don't capture, hence the .+ after it
+like(http_get('/nocache_var'), qr/"(.+)\/(?!\1).+"/, 'noncacheable variable');
+
+TODO: {
+    local $TODO = "Needs upstream nginx patch https://mailman.nginx.org/pipermail/nginx-devel/2024-August/N7VFIYUKSZFUIAO24OJODKQGTP63R5NV.html";
+
+    # Without the patch, this will give 2/4 (calls are duplicated)
+    like(http_get('/callcount_var'), qr/"1\/2"/, 'callcount variable');
+}
+
+###############################################################################
diff --git a/nginx/t/stream_js_variables_nocache.t b/nginx/t/stream_js_variables_nocache.t
new file mode 100644
index 00000000..a9e49c9f
--- /dev/null
+++ b/nginx/t/stream_js_variables_nocache.t
@@ -0,0 +1,94 @@
+#!/usr/bin/perl
+
+# (C) Thomas P.
+
+# Tests for stream njs module, setting non-cacheable nginx variables.
+
+###############################################################################
+
+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_set $nocache_var   test.variable nocache;
+    js_set $default_var   test.variable;
+    js_set $callcount_var test.callCount nocache;
+
+    js_import test.js;
+
+    server {
+        listen  127.0.0.1:8081;
+        set     $a $default_var;
+        set     $b $default_var;
+        return  '"$a/$b"';
+    }
+
+    server {
+        listen  127.0.0.1:8082;
+        set     $a $nocache_var;
+        set     $b $nocache_var;
+        return  '"$a/$b"';
+    }
+
+    server {
+        listen  127.0.0.1:8083;
+        set     $a $callcount_var;
+        set     $b $callcount_var;
+        return  '"$a/$b"';
+    }
+}
+
+EOF
+
+$t->write_file('test.js', <<EOF);
+    function variable(r) {
+        return Math.random().toFixed(16);
+    }
+
+    let n = 0;
+    function callCount(r) {
+        return (n += 1).toString();
+    }
+
+    export default {variable, callCount};
+
+EOF
+
+$t->try_run('no nocache stream njs variables')->plan(3);
+
+###############################################################################
+
+# We use backreferences to make sure the same value was returned for the two uses
+like(stream('127.0.0.1:' . port(8081))->read(), qr/"(.+)\/\1"/, 'cached variable');
+# Negative lookaheads don't capture, hence the .+ after it
+like(stream('127.0.0.1:' . port(8082))->read(), qr/"(.+)\/(?!\1).+"/, 'noncacheable variable');
+like(stream('127.0.0.1:' . port(8083))->read(), qr/"1\/2"/, 'callcount variable');
+
+$t->stop();
+
+###############################################################################


More information about the nginx-devel mailing list