Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

QuickJS modules. #779

Merged
merged 3 commits into from
Sep 18, 2024
Merged

QuickJS modules. #779

merged 3 commits into from
Sep 18, 2024

Conversation

xeioex
Copy link
Contributor

@xeioex xeioex commented Aug 31, 2024

Two main patches:

  1. Modules: introduced engine API. - adapts the modules code to allow multiple JS engines.

  2. Modules: introduced QuickJS engine.
    "js_engine" directive is introduced which sets JavaScript engine.
    When the module is built with QuickJS library "js_engine qjs;" sets
    QuickJS engine for the current block. By default njs engine is used.

    For example,

    nginx.conf:

     location /a {
         js_engine qjs;
         # will be handled by QuickJS
         js_content main.handler;
     }
    
     location /b {
         # will be handled by njs
         js_content main.handler;
     }
    

    QuickJS engine implements drop-in replacement for nginx/njs objects
    with the following exceptions:

     * nginx module API to be added later: ngx.fetch(), ngx.shared.dict.
     * Built-in modules to be added later: fs, crypto, WebCrypto, xml.
     * NJS specific API: njs.dump(), njs.on(), console.dump().
     * js_preload_object directive.
    

Use TEST_NGINX_GLOBALS_HTTP='js_engine qjs;' TEST_NGINX_GLOBALS_STREAM='js_engine qjs;' when running njs tests to test QuickJS module.
For example:
to run tests with njs engine: TEST_NGINX_BINARY=/path/to/nginx/binary prove -I /path/to/nginx-tests/lib/ nginx/t/
to run tests with QuickJS engine: TEST_NGINX_GLOBALS_HTTP='js_engine qjs;' TEST_NGINX_GLOBALS_STREAM='js_engine qjs;' TEST_NGINX_BINARY=/path/to/nginx/binary prove -I /path/to/nginx-tests/lib/ nginx/t/

@xeioex xeioex changed the title Quickjs modules. QuickJS modules. Aug 31, 2024
nginx/t/js_engine.t Outdated Show resolved Hide resolved
nginx/ngx_js.h Outdated Show resolved Hide resolved
nginx/ngx_js.h Outdated Show resolved Hide resolved
@xeioex xeioex force-pushed the quickjs-modules branch 18 times, most recently from f922771 to e62e638 Compare September 7, 2024 06:16
@VadimZhestikov
Copy link
Contributor

Issues found with latest quickjs and nginx with memory sanitizer.
There is small test for 2 memory related issues.

#!/usr/bin/perl

###############################################################################

use warnings;
use strict;

use Test::More;
use Socket qw/ CRLF /;

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_import test.js;

server {
    listen       127.0.0.1:8080;
    server_name  localhost;


    location /request_body_cache {
        js_content test.request_body_cache;
    }

}

}

EOF

$t->write_file('test.js', <<EOF);

function request_body_cache(r) {

    // signal 6
    r.requestBuffer;
    r.requestText;
    r.return(200, "requestText:string requestBuffer:buffer");

    //// Direct leak of 25 byte(s), it corresponds to 16bytes_value + "REQ-BODY"_length_including_trainling_0
    //r.requestText;
    //r.requestBuffer;
    //r.return(200, "RETURN");

}

export default {request_body_cache};

EOF

$t->try_run('no njs available')->plan(1);

###############################################################################

like(http_post('/request_body_cache'),
<------>qr/RETURN$/s, 'request body cache');

$t->stop();

###############################################################################

sub http_post {
<------>my ($url, %extra) = @_;

<------>my $p = "POST $url HTTP/1.0" . CRLF .
<------><------>"Host: localhost" . CRLF .
<------><------>"Content-Length: 8" . CRLF .
<------><------>CRLF .
<------><------>"REQ-BODY";

<------>return http($p, %extra);
}

###############################################################################

Direct leak:

=================================================================
==299884==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 25 byte(s) in 1 object(s) allocated from:
#0 0x7f5a10708887 in __interceptor_malloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:145
#1 0x56160ed3c92a in js_def_malloc /home/vadim/fixes.github/quickjs/quickjs.c:1729
#2 0x56160eda2981 in js_malloc_rt /home/vadim/fixes.github/quickjs/quickjs.c:1316
#3 0x56160eda2981 in js_alloc_string_rt /home/vadim/fixes.github/quickjs/quickjs.c:1895
#4 0x56160eda2981 in js_alloc_string /home/vadim/fixes.github/quickjs/quickjs.c:1913
#5 0x56160edb6eb1 in js_new_string8 /home/vadim/fixes.github/quickjs/quickjs.c:3492
#6 0x56160edb2846 in JS_NewStringLen /home/vadim/fixes.github/quickjs/quickjs.c:3903
#7 0x56160ecc4329 in ngx_http_qjs_ext_request_body /home/vadim/fixes.hg/011530-atomics/njs_plus_nginx_tests_2/njs-2/nginx/ngx_http_js_module.c:5300
#8 0x56160ee2b178 in js_call_c_function /home/vadim/fixes.github/quickjs/quickjs.c:16059
#9 0x56160ed61ccb in JS_CallInternal /home/vadim/fixes.github/quickjs/quickjs.c:16231
#10 0x56160ed87969 in JS_CallFree /home/vadim/fixes.github/quickjs/quickjs.c:18717
#11 0x56160ed8850a in JS_GetPropertyInternal /home/vadim/fixes.github/quickjs/quickjs.c:7199
#12 0x56160ed759ea in JS_GetProperty /home/vadim/fixes.github/quickjs/quickjs.h:741
#13 0x56160ed759ea in JS_CallInternal /home/vadim/fixes.github/quickjs/quickjs.c:17516
#14 0x56160ee95045 in JS_Call /home/vadim/fixes.github/quickjs/quickjs.c:18710
#15 0x56160ecd906b in ngx_engine_qjs_call /home/vadim/fixes.hg/011530-atomics/njs_plus_nginx_tests_2/njs-2/nginx/ngx_js.c:1050
#16 0x56160ecb01e7 in ngx_http_js_content_event_handler /home/vadim/fixes.hg/011530-atomics/njs_plus_nginx_tests_2/njs-2/nginx/ngx_http_js_module.c:1203
#17 0x56160eb89e3b in ngx_http_read_client_request_body src/http/ngx_http_request_body.c:162
#18 0x56160ecafe73 in ngx_http_js_content_handler /home/vadim/fixes.hg/011530-atomics/njs_plus_nginx_tests_2/njs-2/nginx/ngx_http_js_module.c:1160
#19 0x56160eb45865 in ngx_http_core_content_phase src/http/ngx_http_core_module.c:1261
#20 0x56160eb42846 in ngx_http_core_run_phases src/http/ngx_http_core_module.c:875
#21 0x56160eb426fb in ngx_http_handler src/http/ngx_http_core_module.c:858
#22 0x56160eb6c2f6 in ngx_http_process_request src/http/ngx_http_request.c:2140
#23 0x56160eb68a7b in ngx_http_process_request_headers src/http/ngx_http_request.c:1529
#24 0x56160eb66800 in ngx_http_process_request_line src/http/ngx_http_request.c:1196
#25 0x56160eb64adf in ngx_http_wait_request_handler src/http/ngx_http_request.c:523
#26 0x56160eb03556 in ngx_epoll_process_events src/event/modules/ngx_epoll_module.c:901
#27 0x56160eacc5d0 in ngx_process_events_and_timers src/event/ngx_event.c:248
#28 0x56160eafb2e5 in ngx_worker_process_cycle src/os/unix/ngx_process_cycle.c:721
#29 0x56160eaf27fb in ngx_spawn_process src/os/unix/ngx_process.c:199
#30 0x56160eaf831f in ngx_start_worker_processes src/os/unix/ngx_process_cycle.c:344
#31 0x56160eaf7131 in ngx_master_process_cycle src/os/unix/ngx_process_cycle.c:130
#32 0x56160ea3cebe in main src/core/nginx.c:384

signal 6 is from assertion:
9:38
quickjs.c:1998: JS_FreeRuntime: Assertion `list_empty(&rt->gc_obj_list)' failed.

@xeioex xeioex force-pushed the quickjs-modules branch 5 times, most recently from efbf18f to 5193dd0 Compare September 10, 2024 17:35
@VadimZhestikov
Copy link
Contributor

looks like we have few issues:
js_console-my0.txt

@xeioex xeioex force-pushed the quickjs-modules branch 2 times, most recently from da19272 to ab19fef Compare September 11, 2024 16:27
@xeioex xeioex force-pushed the quickjs-modules branch 3 times, most recently from 5af3730 to ceec08b Compare September 12, 2024 05:00
While it is useful for debugging what objects are leaking at the end
this flag hides QuickJS JSString leaks from Address Sanitizer.
No functional changes.
VadimZhestikov
VadimZhestikov previously approved these changes Sep 14, 2024
@thresheek
Copy link
Member

There are some build/test breakages on select buildbot platforms with this PR, please hold off merging as of 27f70ecd8. Investigating...

VadimZhestikov
VadimZhestikov previously approved these changes Sep 16, 2024
VadimZhestikov
VadimZhestikov previously approved these changes Sep 17, 2024
"js_engine" directive is introduced which sets JavaScript engine.
When the module is built with QuickJS library "js_engine qjs;" sets
QuickJS engine for the current block. By default njs engine is used.

For example,

nginx.conf:

    location /a {
        js_engine qjs;
        # will be handled by QuickJS
        js_content main.handler;
    }

    location /b {
        # will be handled by njs
        js_content main.handler;
    }

QuickJS engine implements drop-in replacement for nginx/njs objects
with the following exceptions:

    * nginx module API to be added later: ngx.fetch(), ngx.shared.dict.
    * Built-in modules to be added later: fs, crypto, WebCrypto, xml.
    * NJS specific API: njs.dump(), njs.on(), console.dump().
    * js_preload_object directive.
@xeioex xeioex merged commit 201b127 into nginx:master Sep 18, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants