[njs] Modules: fixed js_set with Buffer values.

Dmitry Volyntsev xeioex at nginx.com
Tue Nov 21 16:58:01 UTC 2023


details:   https://hg.nginx.org/njs/rev/1d13f6e877ad
branches:  
changeset: 2236:1d13f6e877ad
user:      Dmitry Volyntsev <xeioex at nginx.com>
date:      Tue Nov 21 08:57:03 2023 -0800
description:
Modules: fixed js_set with Buffer values.

Previously, a Buffer value which contains invalid UTF-8 when returned as a
value for js_set handler was mangled because the bytes value was converted to a
string value.

The fix is to use bytes value of Buffer, TypedArray and ArrayBuffer as is,
and not convert it to a string first.

diffstat:

 nginx/ngx_http_js_module.c   |   8 +++---
 nginx/ngx_stream_js_module.c |   8 +++---
 nginx/t/js.t                 |  28 ++++++++++++++++++++++++--
 nginx/t/stream_js.t          |  45 +++++++++++++++++++++++++++++++++++++++++--
 4 files changed, 75 insertions(+), 14 deletions(-)

diffs (243 lines):

diff -r 8e024f36e38e -r 1d13f6e877ad nginx/ngx_http_js_module.c
--- a/nginx/ngx_http_js_module.c	Tue Nov 21 06:49:30 2023 -0800
+++ b/nginx/ngx_http_js_module.c	Tue Nov 21 08:57:03 2023 -0800
@@ -1250,7 +1250,7 @@ ngx_http_js_variable_set(ngx_http_reques
 
     ngx_int_t           rc;
     njs_int_t           pending;
-    ngx_str_t           value;
+    njs_str_t           value;
     ngx_http_js_ctx_t  *ctx;
 
     rc = ngx_http_js_init_vm(r, ngx_http_js_request_proto_id);
@@ -1285,15 +1285,15 @@ ngx_http_js_variable_set(ngx_http_reques
         return NGX_ERROR;
     }
 
-    if (ngx_js_retval(ctx->vm, &ctx->retval, &value) != NGX_OK) {
+    if (ngx_js_string(ctx->vm, njs_value_arg(&ctx->retval), &value) != NGX_OK) {
         return NGX_ERROR;
     }
 
-    v->len = value.len;
+    v->len = value.length;
     v->valid = 1;
     v->no_cacheable = 0;
     v->not_found = 0;
-    v->data = value.data;
+    v->data = value.start;
 
     return NGX_OK;
 }
diff -r 8e024f36e38e -r 1d13f6e877ad nginx/ngx_stream_js_module.c
--- a/nginx/ngx_stream_js_module.c	Tue Nov 21 06:49:30 2023 -0800
+++ b/nginx/ngx_stream_js_module.c	Tue Nov 21 08:57:03 2023 -0800
@@ -914,7 +914,7 @@ ngx_stream_js_variable_set(ngx_stream_se
 
     ngx_int_t             rc;
     njs_int_t             pending;
-    ngx_str_t             value;
+    njs_str_t             value;
     ngx_stream_js_ctx_t  *ctx;
 
     rc = ngx_stream_js_init_vm(s, ngx_stream_js_session_proto_id);
@@ -949,15 +949,15 @@ ngx_stream_js_variable_set(ngx_stream_se
         return NGX_ERROR;
     }
 
-    if (ngx_js_retval(ctx->vm, &ctx->retval, &value) != NGX_OK) {
+    if (ngx_js_string(ctx->vm, njs_value_arg(&ctx->retval), &value) != NGX_OK) {
         return NGX_ERROR;
     }
 
-    v->len = value.len;
+    v->len = value.length;
     v->valid = 1;
     v->no_cacheable = 0;
     v->not_found = 0;
-    v->data = value.data;
+    v->data = value.start;
 
     return NGX_OK;
 }
diff -r 8e024f36e38e -r 1d13f6e877ad nginx/t/js.t
--- a/nginx/t/js.t	Tue Nov 21 06:49:30 2023 -0800
+++ b/nginx/t/js.t	Tue Nov 21 08:57:03 2023 -0800
@@ -46,6 +46,7 @@ http {
     js_set $test_global   test.global_obj;
     js_set $test_log      test.log;
     js_set $test_internal test.sub_internal;
+    js_set $buffer        test.buffer;
     js_set $test_except   test.except;
 
     js_import test.js;
@@ -95,6 +96,10 @@ http {
             js_content test.status;
         }
 
+        location /buffer_variable {
+            js_content test.buffer_variable;
+        }
+
         location /request_body {
             js_content test.request_body;
         }
@@ -171,6 +176,10 @@ EOF
         return 'variable=' + r.variables.remote_addr;
     }
 
+    function buffer(r) {
+        return Buffer.from([0xaa, 0xbb, 0xcc, 0xdd]);
+    }
+
     function global_obj(r) {
         return 'global=' + global;
     }
@@ -225,6 +234,10 @@ EOF
         r.log('SEE-LOG');
     }
 
+    function buffer_variable(r) {
+        r.return(200, r.rawVariables.buffer.toString('hex'));
+    }
+
     async function internal(r) {
         let reply = await r.subrequest('/sub_internal');
 
@@ -248,14 +261,15 @@ EOF
     function content_empty(r) {
     }
 
-    export default {njs:test_njs, method, version, addr, uri,
+    export default {njs:test_njs, method, version, addr, uri, buffer,
                     variable, global_obj, status, request_body, internal,
                     request_body_cache, send, return_method, sub_internal,
-                    type, log, except, content_except, content_empty};
+                    type, log, buffer_variable, except, content_except,
+                    content_empty};
 
 EOF
 
-$t->try_run('no njs available')->plan(27);
+$t->try_run('no njs available')->plan(28);
 
 ###############################################################################
 
@@ -307,6 +321,14 @@ like(http_get('/internal'), qr/parent: f
 
 }
 
+
+TODO: {
+local $TODO = 'not yet' unless has_version('0.8.3');
+
+like(http_get('/buffer_variable'), qr/aabbccdd/, 'buffer variable');
+
+}
+
 http_get('/except');
 http_get('/content_except');
 
diff -r 8e024f36e38e -r 1d13f6e877ad nginx/t/stream_js.t
--- a/nginx/t/stream_js.t	Tue Nov 21 06:49:30 2023 -0800
+++ b/nginx/t/stream_js.t	Tue Nov 21 08:57:03 2023 -0800
@@ -68,6 +68,7 @@ stream {
     js_set $js_req_line  test.req_line;
     js_set $js_sess_unk  test.sess_unk;
     js_set $js_async     test.asyncf;
+    js_set $js_buffer    test.buffer;
 
     js_import test.js;
 
@@ -190,6 +191,11 @@ stream {
         listen      127.0.0.1:8100;
         return      $js_async;
     }
+
+    server {
+        listen      127.0.0.1:8101;
+        return      $js_buffer;
+    }
 }
 
 EOF
@@ -211,8 +217,13 @@ EOF
         return 'sess_unk=' + s.unk;
     }
 
+    function buffer(s) {
+        return Buffer.from([0xaa, 0xbb, 0xcc, 0xdd]);
+    }
+
     function log(s) {
         s.log("SEE-THIS");
+        return 'log';
     }
 
     var res = '';
@@ -377,12 +388,13 @@ EOF
                     preread_step, filter_step, access_undecided, access_allow,
                     access_deny, preread_async, preread_data, preread_req_line,
                     req_line, filter_empty, filter_header_inject, filter_search,
-                    access_except, preread_except, filter_except, asyncf};
+                    access_except, preread_except, filter_except, asyncf,
+                    buffer};
 
 EOF
 
 $t->run_daemon(\&stream_daemon, port(8090));
-$t->try_run('no stream njs available')->plan(23);
+$t->try_run('no stream njs available')->plan(24);
 $t->waitforsocket('127.0.0.1:' . port(8090));
 
 ###############################################################################
@@ -391,7 +403,7 @@ is(stream('127.0.0.1:' . port(8080))->re
 	's.remoteAddress');
 is(dgram('127.0.0.1:' . port(8985))->io('.'), 'addr=127.0.0.1',
 	's.remoteAddress udp');
-is(stream('127.0.0.1:' . port(8081))->read(), 'undefined', 's.log');
+is(stream('127.0.0.1:' . port(8081))->read(), 'log', 's.log');
 is(stream('127.0.0.1:' . port(8082))->read(), 'variable=127.0.0.1',
 	's.variables');
 is(stream('127.0.0.1:' . port(8083))->read(), '', 'stream js unknown function');
@@ -417,6 +429,14 @@ stream('127.0.0.1:' . port(8099))->io('x
 
 is(stream('127.0.0.1:' . port(8100))->read(), 'retval: 30', 'asyncf');
 
+TODO: {
+local $TODO = 'not yet' unless has_version('0.8.3');
+
+like(stream('127.0.0.1:' . port(8101))->read(), qr/\xaa\xbb\xcc\xdd/,
+	'buffer variable');
+
+}
+
 $t->stop();
 
 ok(index($t->read_file('error.log'), 'SEE-THIS') > 0, 'stream js log');
@@ -432,6 +452,25 @@ like($t->read_file('status.log'), qr/$p[
 
 ###############################################################################
 
+sub has_version {
+	my $need = shift;
+
+	get('/njs') =~ /^([.0-9]+)$/m;
+
+	my @v = split(/\./, $1);
+	my ($n, $v);
+
+	for $n (split(/\./, $need)) {
+		$v = shift @v || 0;
+		return 0 if $n > $v;
+		return 1 if $v > $n;
+	}
+
+	return 1;
+}
+
+###############################################################################
+
 sub stream_daemon {
 	my $server = IO::Socket::INET->new(
 		Proto => 'tcp',


More information about the nginx-devel mailing list