Modules: fix subrequest response body leak in QuickJS

Previously, alternating r.responseText and r.responseBuffer on a
subrequest reply overwrote the cached req->response_body value
without freeing it.  The value stayed referenced until the runtime
was freed, triggering the "list_empty(&rt->gc_obj_list)" assertion
in JS_FreeRuntime() in debug builds and accumulating in the runtime
with js_context_reuse.

The fix is to free the cached value before replacing it, as is
already done for r.requestText and r.requestBuffer.

The issue was introduced in 201b1276 (0.8.6).

diff --git a/nginx/ngx_http_js_module.c b/nginx/ngx_http_js_module.c
index 0c00ed6530aabd3018e2e6f06948d34f98a2254e..86399ec001d1cb0470c4a3faeeffc60bd292910c 100644 (file)
--- a/nginx/ngx_http_js_module.c
+++ b/nginx/ngx_http_js_module.c
@@ -6597,6 +6597,9 @@ ngx_http_qjs_ext_response_body(JSContext *cx, JSValueConst this_val, int type)
         if ((buffer_type == NGX_JS_STRING) == JS_IsString(req->response_body)) {
             return JS_DupValue(cx, req->response_body);
         }
+
+        JS_FreeValue(cx, req->response_body);
+        req->response_body = JS_UNDEFINED;
     }
 
     r = req->request;

diff --git a/nginx/t/js_subrequests.t b/nginx/t/js_subrequests.t
index bebc7c5116610b02fa0c3f9f80644269bde2982b..eaabf7cd878c1cdad4a7d1aa6e751438234d095f 100644 (file)
--- a/nginx/t/js_subrequests.t
+++ b/nginx/t/js_subrequests.t
@@ -100,6 +100,10 @@ http {
             js_content test.sr_body_pr;
         }
 
+        location /sr_body_types {
+            js_content test.sr_body_types;
+        }
+
         location /sr_body_special {
             js_content test.sr_body_special;
         }
@@ -339,6 +343,15 @@ $t->write_file('test.js', <<EOF);
         .then(body_fwd_cb);
     }
 
+    function sr_body_types(r) {
+        r.subrequest('/js_sub', reply => {
+            let body = [reply.responseText, reply.responseBuffer,
+                        reply.responseText, reply.responseBuffer];
+
+            r.return(200, '[' + body.map(v => v.toString()).join(',') + ']');
+        });
+    }
+
     function sr_body_special(r) {
         r.subrequest('/p/sub2', body_fwd_cb);
     }
@@ -524,7 +537,8 @@ $t->write_file('test.js', <<EOF);
     export default {njs:test_njs, sr, sr_pr, sr_args, sr_options_args,
                     sr_options_args_pr, sr_options_method, sr_options_method_pr,
                     sr_options_method_head, sr_options_body, sr_body,
-                    sr_body_pr, sr_body_special, body, delayed, detached,
+                    sr_body_pr, sr_body_types, sr_body_special, body, delayed,
+                    detached,
                     sr_in_variable_handler, async_var, sr_error_page,
                     subrequest_var, sr_file, sr_cache, sr_unavail, sr_parent,
                     sr_unavail_pr, sr_broken, sr_too_large, sr_in_sr,
@@ -537,7 +551,7 @@ EOF
 
 $t->write_file('t', '["SEE-THIS"]');
 
-$t->try_run('no njs available')->plan(34);
+$t->try_run('no njs available')->plan(35);
 $t->run_daemon(\&http_daemon);
 
 ###############################################################################
@@ -550,6 +564,9 @@ is(get_json('/sr_options_method?m=PURGE'), '["PURGE"]', 'sr method PURGE');
 is(get_json('/sr_options_body'), '["REQ-BODY"]', 'sr_options_body');
 is(get_json('/sr_options_method_head'), '{"c":200}', 'sr_options_method_head');
 is(get_json('/sr_body'), '{"a":{"b":1}}', 'sr_body');
+is(get_json('/sr_body_types'),
+   '[["JS-SUB"],["JS-SUB"],["JS-SUB"],["JS-SUB"]]',
+   'subrequest responseText and responseBuffer');
 is(get_json('/sr_body_special'), '{"e":"msg"}', 'sr_body_special');
 is(get_json('/sr_in_variable_handler'), '["CB-VAR"]', 'sr_in_variable_handler');
 is(get_json('/sr_file'), '["SEE-THIS"]', 'sr_file');