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.

diff --git a/nginx/ngx_http_js_module.c b/nginx/ngx_http_js_module.c
index 49f9790f04d189c006eb9e52070ae0668b5bb3f5..6c74a4e57614564bce6252d55865084e38f7df54 100644 (file)
--- a/nginx/ngx_http_js_module.c
+++ b/nginx/ngx_http_js_module.c
@@ -1250,7 +1250,7 @@ ngx_http_js_variable_set(ngx_http_request_t *r, ngx_http_variable_value_t *v,
 
     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_request_t *r, ngx_http_variable_value_t *v,
         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 --git a/nginx/ngx_stream_js_module.c b/nginx/ngx_stream_js_module.c
index 1540d4b34cb02013e48636274a603f4e11d20c97..1a36d4158eb07ddc60878110fa01fa1c0baa2035 100644 (file)
--- a/nginx/ngx_stream_js_module.c
+++ b/nginx/ngx_stream_js_module.c
@@ -914,7 +914,7 @@ ngx_stream_js_variable_set(ngx_stream_session_t *s,
 
     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_session_t *s,
         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 --git a/nginx/t/js.t b/nginx/t/js.t
index 5f2dcda87e5d024aba097a329415c96f9e3b93a6..d7a5a0c50749d10c97502635395d784a0c850e97 100644 (file)
--- a/nginx/t/js.t
+++ b/nginx/t/js.t
@@ -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 @@ $t->write_file('test.js', <<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 @@ $t->write_file('test.js', <<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 @@ $t->write_file('test.js', <<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: false sub: true/, 'r.internal');
 
 }
 
+
+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 --git a/nginx/t/stream_js.t b/nginx/t/stream_js.t
index ff92aad269b362b14db6f4f4d933e4eb6b2dd1a9..8bf0dd375a4ea6558dfe591120a15842ee1c47bd 100644 (file)
--- a/nginx/t/stream_js.t
+++ b/nginx/t/stream_js.t
@@ -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 @@ $t->write_file('test.js', <<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 @@ $t->write_file('test.js', <<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))->read(), 'addr=127.0.0.1',
        '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[2]:403/, 'status deny');
 
 ###############################################################################
 
+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',