HTTP: fixed r.return() with empty string as a body argument.

nginx · Sep 26, 2024 · 6e6a9c5 · 6e6a9c5
1 parent c2bc8c6
commit 6e6a9c5
Show file tree

Hide file tree

Showing 2 changed files with 48 additions and 12 deletions.
diff --git a/nginx/ngx_http_js_module.c b/nginx/ngx_http_js_module.c
@@ -2704,14 +2704,16 @@ ngx_http_js_ext_return(njs_vm_t *vm, njs_value_t *args, njs_uint_t nargs,
         return NJS_ERROR;
     }
 
-    if (ngx_js_string(vm, njs_arg(args, nargs, 2), &text) != NGX_OK) {
-        njs_vm_error(vm, "failed to convert text");
-        return NJS_ERROR;
-    }
-
     ctx = ngx_http_get_module_ctx(r, ngx_http_js_module);
 
-    if (status < NGX_HTTP_BAD_REQUEST || text.length) {
+    if (status < NGX_HTTP_BAD_REQUEST
+        || !njs_value_is_null_or_undefined(njs_arg(args, nargs, 2)))
+    {
+        if (ngx_js_string(vm, njs_arg(args, nargs, 2), &text) != NGX_OK) {
+            njs_vm_error(vm, "failed to convert text");
+            return NJS_ERROR;
+        }
+
         ngx_memzero(&cv, sizeof(ngx_http_complex_value_t));
 
         cv.value.data = text.start;
@@ -5352,11 +5354,11 @@ ngx_http_qjs_ext_return(JSContext *cx, JSValueConst this_val,
 
     ctx = ngx_http_get_module_ctx(r, ngx_http_js_module);
 
-    if (ngx_qjs_string(ctx->engine, argv[1], &body) != NGX_OK) {
-        return JS_ThrowOutOfMemory(cx);
-    }
+    if (status < NGX_HTTP_BAD_REQUEST || !JS_IsNullOrUndefined(argv[1])) {
+        if (ngx_qjs_string(ctx->engine, argv[1], &body) != NGX_OK) {
+            return JS_ThrowOutOfMemory(cx);
+        }
 
-    if (status < NGX_HTTP_BAD_REQUEST || body.len) {
         ngx_memzero(&cv, sizeof(ngx_http_complex_value_t));
 
         cv.value.data = body.data;

diff --git a/nginx/t/js_return.t b/nginx/t/js_return.t
@@ -46,21 +46,29 @@ http {
         location / {
             js_content test.returnf;
         }
+
+        location /njs {
+            js_content test.njs;
+        }
     }
 }
 
 EOF
 
 $t->write_file('test.js', <<EOF);
+    function test_njs(r) {
+        r.return(200, njs.version);
+    }
+
     function returnf(r) {
         r.return(Number(r.args.c), r.args.t);
     }
 
-    export default {returnf};
+    export default {njs:test_njs, returnf};
 
 EOF
 
-$t->try_run('no njs return')->plan(5);
+$t->try_run('no njs return')->plan(6);
 
 ###############################################################################
 
@@ -70,4 +78,30 @@ like(http_get('/?c=301&t=path'), qr/ 301 .*Location: path/s, 'return redirect');
 like(http_get('/?c=404'), qr/404 Not.*html/s, 'return error page');
 like(http_get('/?c=inv'), qr/ 500 /, 'return invalid');
 
+TODO: {
+local $TODO = 'not yet' unless has_version('0.8.6');
+
+unlike(http_get('/?c=404&t='), qr/Not.*html/s, 'return empty body');
+
+}
+
+###############################################################################
+
+sub has_version {
+	my $need = shift;
+
+	http_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;
+}
+
 ###############################################################################