HTTP: fixed limit rated output.

Previously, when r.return(code, body) was called from a subrequest handler with a body size larger than the sendfile_max_chunk value connection hanging might occur.
nginx · Nov 27, 2024 · b300a93 · b300a93
1 parent 283282f
commit b300a93
Show file tree

Hide file tree

Showing 3 changed files with 44 additions and 6 deletions.
diff --git a/nginx/ngx_http_js_module.c b/nginx/ngx_http_js_module.c
@@ -1230,14 +1230,17 @@ ngx_http_js_content_write_event_handler(ngx_http_request_t *r)
     ngx_log_debug0(NGX_LOG_DEBUG_HTTP, r->connection->log, 0,
                    "http js content write event handler");
 
+    c = r->connection;
     ctx = ngx_http_get_module_ctx(r, ngx_http_js_module);
 
     if (!ngx_js_ctx_pending(ctx)) {
         ngx_http_js_content_finalize(r, ctx);
-        return;
+
+        if (!c->buffered) {
+            return;
+        }
     }
 
-    c = r->connection;
     wev = c->write;
 
     if (wev->timedout) {

diff --git a/nginx/t/js_return.t b/nginx/t/js_return.t
@@ -47,6 +47,11 @@ http {
             js_content test.returnf;
         }
 
+        location /limit {
+            sendfile_max_chunk 5;
+            js_content test.returnf;
+        }
+
         location /njs {
             js_content test.njs;
         }
@@ -61,14 +66,19 @@ $t->write_file('test.js', <<EOF);
     }
 
     function returnf(r) {
-        r.return(Number(r.args.c), r.args.t);
+        let body = r.args.t;
+        if (body && r.args.repeat) {
+            body = body.repeat(r.args.repeat);
+        }
+
+        r.return(Number(r.args.c), body);
     }
 
     export default {njs:test_njs, returnf};
 
 EOF
 
-$t->try_run('no njs return')->plan(6);
+$t->try_run('no njs return')->plan(7);
 
 ###############################################################################
 
@@ -85,6 +95,14 @@ unlike(http_get('/?c=404&t='), qr/Not.*html/s, 'return empty body');
 
 }
 
+TODO: {
+local $TODO = 'not yet' unless has_version('0.8.8');
+
+like(http_get('/limit?c=200&t=X&repeat=50'), qr/200 OK.*X{50}/s,
+	'return limited');
+
+}
+
 ###############################################################################
 
 sub has_version {

diff --git a/nginx/t/js_subrequests.t b/nginx/t/js_subrequests.t
@@ -139,6 +139,11 @@ http {
             js_content test.sr_cache;
         }
 
+        location /sr_limit {
+            sendfile_max_chunk 5;
+            js_content test.sr_limit;
+        }
+
 
         location /sr_unavail {
             js_content test.sr_unavail;
@@ -385,6 +390,12 @@ $t->write_file('test.js', <<EOF);
         r.subrequest('/p/t', body_fwd_cb);
     }
 
+    function sr_limit(r) {
+        r.subrequest('/file/t', function (reply) {
+            r.return(200, "x".repeat(100));
+        });
+    }
+
     function sr_unavail(req) {
         subrequest_fn(req, ['/unavail'], ['status']);
     }
@@ -520,13 +531,13 @@ $t->write_file('test.js', <<EOF);
                     sr_js_in_subrequest, sr_js_in_subrequest_pr, js_sub,
                     sr_in_sr_callback, sr_out_of_order, sr_except_not_a_func,
                     sr_uri_except, sr_except_failed_to_convert_options_arg,
-                    sr_unsafe, sr_error_in_callback};
+                    sr_unsafe, sr_error_in_callback, sr_limit};
 
 EOF
 
 $t->write_file('t', '["SEE-THIS"]');
 
-$t->try_run('no njs available')->plan(33);
+$t->try_run('no njs available')->plan(34);
 $t->run_daemon(\&http_daemon);
 
 ###############################################################################
@@ -597,6 +608,12 @@ ok(index($t->read_file('error.log'), 'subrequest can only be created for') > 0,
 
 }
 
+TODO: {
+local $TODO = 'not yet' unless has_version('0.8.8');
+
+like(http_get('/sr_limit'), qr/x{100}/, 'sr_limit');
+}
+
 $t->stop();
 
 ok(index($t->read_file('error.log'), 'callback is not a function') > 0,