diff --git a/src/ngx_http_lua_subrequest.c b/src/ngx_http_lua_subrequest.c index c9a5c08622..007f0705e3 100644 --- a/src/ngx_http_lua_subrequest.c +++ b/src/ngx_http_lua_subrequest.c @@ -609,6 +609,10 @@ ngx_http_lua_ngx_location_capture_multi(lua_State *L) return luaL_error(L, "failed to issue subrequest: %d", (int) rc); } + #if (NGX_HTTP_OPENRESTY_LUA_SUBREQUEST) + sr->lua_subrequest = 1; + #endif + ngx_http_lua_init_ctx(sr, sr_ctx); sr_ctx->capture = 1; diff --git a/t/190-http2-slice-subreq-timeout.t b/t/190-http2-slice-subreq-timeout.t new file mode 100644 index 0000000000..acb07fe941 --- /dev/null +++ b/t/190-http2-slice-subreq-timeout.t @@ -0,0 +1,199 @@ +# vim:set ft= ts=4 sw=4 et fdm=marker: +# +# Regression test for openresty/openresty#1131. +# The http2_subreq_error_wakeup patch must not skip request termination for +# native nginx subrequests, such as slice range subrequests, which do not have +# a post_subrequest callback waiting to resume a parent coroutine. +# +# The test warms a sliced proxy cache first. It then opens a raw h2c client, +# requests the cached sliced file, reads a few slices, and stops reading while +# keeping the connection open. That makes nginx hit send_timeout while the +# active request is a native slice subrequest. A buggy build falls through to +# special response handling and logs "header already sent"; a fixed build +# terminates the native subrequest instead. + +our $SkipReason; + +BEGIN { + my $nginx = $ENV{TEST_NGINX_BINARY} || 'nginx'; + my $nginx_version = `$nginx -V 2>&1`; + + if ($? != 0) { + $SkipReason = "failed to get nginx version"; + + } elsif ($nginx_version !~ /--with-http_slice_module/) { + $SkipReason = "requires nginx built with --with-http_slice_module"; + } +} + +use File::Path qw(remove_tree); +use Test::Nginx::Socket::Lua $SkipReason ? (skip_all => $SkipReason) + : ('no_plan'); + +repeat_each(1); + +no_shuffle(); +no_long_string(); + +our $HtmlDir = html_dir; +$ENV{TEST_NGINX_HTML_DIR} = $HtmlDir; + +our $ServerRoot = server_root(); +our $CacheDir = "$ServerRoot/slice_cache"; +remove_tree($CacheDir); +add_cleanup_handler(sub { remove_tree($CacheDir) }); + +our $HttpConfig = qq{ + proxy_cache_path $CacheDir levels=1:2 keys_zone=slicecache:10m + inactive=10m max_size=50m; + send_timeout 1s; + + upstream slice_origin { + server unix:$HtmlDir/slice-origin.sock; + } + + server { + listen unix:$HtmlDir/slice-origin.sock; + + location / { + root $HtmlDir; + } + } +}; + +run_tests(); + +__DATA__ + +=== TEST 1: stalled HTTP/2 client timeout with cached slice subrequests +--- http_config eval: $::HttpConfig +--- config + location = /ping { + return 200 "ok\n"; + } + + location = /slice.bin { + send_timeout 1s; + slice 128k; + proxy_cache slicecache; + proxy_cache_key "$uri $slice_range"; + proxy_set_header Range $slice_range; + proxy_cache_valid 200 206 1h; + proxy_pass http://slice_origin; + } +--- init +use IO::Select; +use IO::Socket::INET; + +sub h2_frame { + my ($type, $flags, $sid, $payload) = @_; + my $len = length $payload; + + # Build one HTTP/2 frame: + # 9-byte frame header (length, type, flags, stream id) + payload. + return pack("C3 C C N", + ($len >> 16) & 0xff, + ($len >> 8) & 0xff, + $len & 0xff, + $type, + $flags, + $sid & 0x7fffffff) + . $payload; +} + +sub hpack_headers_for_path { + my ($path) = @_; + + # Minimal HPACK request header block, equivalent to: + # :method: GET + # :scheme: http + # :path: $path + # :authority: localhost + return "\x82\x86" + . "\x04" . chr(length $path) . $path + . "\x01" . "\x09" . "localhost"; +} + +sub run_stalled_h2_client { + my ($port, $path) = @_; + + # Speak h2c on a raw TCP socket so the test can stop reading without curl + # closing the connection or changing the timeout path. + my $sock = IO::Socket::INET->new( + PeerAddr => "127.0.0.1", + PeerPort => $port, + Proto => "tcp", + Timeout => 5, + ) or die "failed to connect to nginx: $!"; + + $sock->autoflush(1); + + my $initial_window = 1024 * 1024; + + # Start an HTTP/2 cleartext session. The larger stream and connection + # windows let nginx send several 128k slices before the client stalls. + print $sock "PRI * HTTP/2.0\r\n\r\nSM\r\n\r\n"; + print $sock h2_frame(0x4, 0x0, 0, pack("nN", 0x4, $initial_window)); + print $sock h2_frame(0x8, 0x0, 0, pack("N", $initial_window)); + + # Send a GET request on stream 1. END_HEADERS | END_STREAM means the + # request has no body. + print $sock h2_frame(0x1, 0x5, 1, hpack_headers_for_path($path)); + + my $sel = IO::Select->new($sock); + my $deadline = time + 5; + my $read = 0; + + # Read about three 128k slices first. This proves the response is already + # flowing and makes the later timeout happen during slice subrequest output. + while ($read < 384 * 1024 && time < $deadline) { + my @ready = $sel->can_read(0.2); + next unless @ready; + + my $n = sysread($sock, my $buf, 65536); + last unless $n; + $read += $n; + } + + # Without this precondition, the timeout might hit a different request + # state and the regression signal would be ambiguous. + die "stalled HTTP/2 client did not receive enough response bytes: $read" + if $read < 384 * 1024; + + # Stop reading for longer than send_timeout (1s) while the connection stays + # open. This is the actual stalled-client trigger. + select undef, undef, undef, 2.5; + close $sock; +} + +# Create a multi-slice origin file and warm proxy_cache with a normal client. +# The stalled h2c request below then runs from cached slice subrequests, making +# the timeout path independent of upstream timing. +my $file = "$::HtmlDir/slice.bin"; +system("dd", "if=/dev/urandom", "of=$file", "bs=1M", "count=2", "status=none") == 0 + or die "failed to create $file"; + +my $port = $Test::Nginx::Util::ServerPortForClient; +my $cmd = "curl -sS --connect-timeout 5 --max-time 30 " + . "http://127.0.0.1:$port/slice.bin -o /dev/null"; +system($cmd) == 0 or die "failed to warm sliced proxy cache: $cmd"; + +run_stalled_h2_client($port, "/slice.bin"); +--- http2 +--- request +GET /ping +--- response_body +ok +--- error_log +http slice subrequest +client timed out +--- no_error_log +header already sent +[alert] +[crit] +[emerg] +--- no_shutdown_error_log +header already sent +[alert] +[crit] +[emerg] diff --git a/util/build-with-dd.sh b/util/build-with-dd.sh index 4f4e785f53..80ea3e2c45 100755 --- a/util/build-with-dd.sh +++ b/util/build-with-dd.sh @@ -18,6 +18,7 @@ time ngx-build $force $version \ --with-ipv6 \ --with-cc-opt="-DNGX_LUA_USE_ASSERT -I$PCRE2_INC -I$OPENSSL_INC -DDDEBUG=1" \ --with-http_v2_module \ + --with-http_slice_module \ --with-http_v3_module \ --with-http_realip_module \ --with-http_ssl_module \ diff --git a/util/build-without-ssl.sh b/util/build-without-ssl.sh index 022d9617e2..715e740b9d 100755 --- a/util/build-without-ssl.sh +++ b/util/build-without-ssl.sh @@ -19,6 +19,7 @@ time ngx-build $force $version \ --with-ipv6 \ --with-cc-opt="-DNGX_LUA_USE_ASSERT -I$PCRE2_INC" \ --with-http_v2_module \ + --with-http_slice_module \ --with-http_realip_module \ --add-module=$root/../ndk-nginx-module \ --add-module=$root/../set-misc-nginx-module \ diff --git a/util/build.sh b/util/build.sh index cd4eaa45f4..670041c012 100755 --- a/util/build.sh +++ b/util/build.sh @@ -32,6 +32,7 @@ time ngx-build $force $version \ --with-ipv6 \ --with-cc-opt="-DNGX_LUA_USE_ASSERT -I$PCRE2_INC -I$OPENSSL_INC" \ --with-http_v2_module \ + --with-http_slice_module \ --with-http_v3_module \ --with-http_realip_module \ --with-http_ssl_module \