From 2743bab180ab067ea6d146c92a30a7f88bff7b2a Mon Sep 17 00:00:00 2001 From: Dmitry Volyntsev Date: Tue, 21 Nov 2023 08:57:03 -0800 Subject: [PATCH] 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. --- 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(-) diff --git a/nginx/ngx_http_js_module.c b/nginx/ngx_http_js_module.c index 49f9790f..6c74a4e5 100644 --- 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 1540d4b3..1a36d415 100644 --- 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 5f2dcda8..d7a5a0c5 100644 --- 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', <write_file('test.js', <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 ff92aad2..8bf0dd37 100644 --- 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', <write_file('test.js', <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', -- 2.47.3