Skip to content

Commit 09b8fe9

Browse files
tzssangglasstzssangglass
authored
bugfix: mark Lua subrequests for HTTP/2 error wakeup (#2508)
Mark capture subrequests so OpenResty's nginx core patch can distinguish Lua subrequests from native nginx subrequests such as slice. This keeps the HTTP/2 error wakeup behavior for ngx.location.capture() without broadening it to unrelated nginx subrequests.
1 parent cf6453c commit 09b8fe9

5 files changed

Lines changed: 206 additions & 0 deletions

File tree

‎src/ngx_http_lua_subrequest.c‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -609,6 +609,10 @@ ngx_http_lua_ngx_location_capture_multi(lua_State *L)
609609
return luaL_error(L, "failed to issue subrequest: %d", (int) rc);
610610
}
611611

612+
#if (NGX_HTTP_OPENRESTY_LUA_SUBREQUEST)
613+
sr->lua_subrequest = 1;
614+
#endif
615+
612616
ngx_http_lua_init_ctx(sr, sr_ctx);
613617

614618
sr_ctx->capture = 1;

‎t/190-http2-slice-subreq-timeout.t‎

Lines changed: 199 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,199 @@
1+
# vim:set ft= ts=4 sw=4 et fdm=marker:
2+
#
3+
# Regression test for openresty/openresty#1131.
4+
# The http2_subreq_error_wakeup patch must not skip request termination for
5+
# native nginx subrequests, such as slice range subrequests, which do not have
6+
# a post_subrequest callback waiting to resume a parent coroutine.
7+
#
8+
# The test warms a sliced proxy cache first. It then opens a raw h2c client,
9+
# requests the cached sliced file, reads a few slices, and stops reading while
10+
# keeping the connection open. That makes nginx hit send_timeout while the
11+
# active request is a native slice subrequest. A buggy build falls through to
12+
# special response handling and logs "header already sent"; a fixed build
13+
# terminates the native subrequest instead.
14+
15+
our $SkipReason;
16+
17+
BEGIN {
18+
my $nginx = $ENV{TEST_NGINX_BINARY} || 'nginx';
19+
my $nginx_version = `$nginx -V 2>&1`;
20+
21+
if ($? != 0) {
22+
$SkipReason = "failed to get nginx version";
23+
24+
} elsif ($nginx_version !~ /--with-http_slice_module/) {
25+
$SkipReason = "requires nginx built with --with-http_slice_module";
26+
}
27+
}
28+
29+
use File::Path qw(remove_tree);
30+
use Test::Nginx::Socket::Lua $SkipReason ? (skip_all => $SkipReason)
31+
: ('no_plan');
32+
33+
repeat_each(1);
34+
35+
no_shuffle();
36+
no_long_string();
37+
38+
our $HtmlDir = html_dir;
39+
$ENV{TEST_NGINX_HTML_DIR} = $HtmlDir;
40+
41+
our $ServerRoot = server_root();
42+
our $CacheDir = "$ServerRoot/slice_cache";
43+
remove_tree($CacheDir);
44+
add_cleanup_handler(sub { remove_tree($CacheDir) });
45+
46+
our $HttpConfig = qq{
47+
proxy_cache_path $CacheDir levels=1:2 keys_zone=slicecache:10m
48+
inactive=10m max_size=50m;
49+
send_timeout 1s;
50+
51+
upstream slice_origin {
52+
server unix:$HtmlDir/slice-origin.sock;
53+
}
54+
55+
server {
56+
listen unix:$HtmlDir/slice-origin.sock;
57+
58+
location / {
59+
root $HtmlDir;
60+
}
61+
}
62+
};
63+
64+
run_tests();
65+
66+
__DATA__
67+
68+
=== TEST 1: stalled HTTP/2 client timeout with cached slice subrequests
69+
--- http_config eval: $::HttpConfig
70+
--- config
71+
location = /ping {
72+
return 200 "ok\n";
73+
}
74+
75+
location = /slice.bin {
76+
send_timeout 1s;
77+
slice 128k;
78+
proxy_cache slicecache;
79+
proxy_cache_key "$uri $slice_range";
80+
proxy_set_header Range $slice_range;
81+
proxy_cache_valid 200 206 1h;
82+
proxy_pass http://slice_origin;
83+
}
84+
--- init
85+
use IO::Select;
86+
use IO::Socket::INET;
87+
88+
sub h2_frame {
89+
my ($type, $flags, $sid, $payload) = @_;
90+
my $len = length $payload;
91+
92+
# Build one HTTP/2 frame:
93+
# 9-byte frame header (length, type, flags, stream id) + payload.
94+
return pack("C3 C C N",
95+
($len >> 16) & 0xff,
96+
($len >> 8) & 0xff,
97+
$len & 0xff,
98+
$type,
99+
$flags,
100+
$sid & 0x7fffffff)
101+
. $payload;
102+
}
103+
104+
sub hpack_headers_for_path {
105+
my ($path) = @_;
106+
107+
# Minimal HPACK request header block, equivalent to:
108+
# :method: GET
109+
# :scheme: http
110+
# :path: $path
111+
# :authority: localhost
112+
return "\x82\x86"
113+
. "\x04" . chr(length $path) . $path
114+
. "\x01" . "\x09" . "localhost";
115+
}
116+
117+
sub run_stalled_h2_client {
118+
my ($port, $path) = @_;
119+
120+
# Speak h2c on a raw TCP socket so the test can stop reading without curl
121+
# closing the connection or changing the timeout path.
122+
my $sock = IO::Socket::INET->new(
123+
PeerAddr => "127.0.0.1",
124+
PeerPort => $port,
125+
Proto => "tcp",
126+
Timeout => 5,
127+
) or die "failed to connect to nginx: $!";
128+
129+
$sock->autoflush(1);
130+
131+
my $initial_window = 1024 * 1024;
132+
133+
# Start an HTTP/2 cleartext session. The larger stream and connection
134+
# windows let nginx send several 128k slices before the client stalls.
135+
print $sock "PRI * HTTP/2.0\r\n\r\nSM\r\n\r\n";
136+
print $sock h2_frame(0x4, 0x0, 0, pack("nN", 0x4, $initial_window));
137+
print $sock h2_frame(0x8, 0x0, 0, pack("N", $initial_window));
138+
139+
# Send a GET request on stream 1. END_HEADERS | END_STREAM means the
140+
# request has no body.
141+
print $sock h2_frame(0x1, 0x5, 1, hpack_headers_for_path($path));
142+
143+
my $sel = IO::Select->new($sock);
144+
my $deadline = time + 5;
145+
my $read = 0;
146+
147+
# Read about three 128k slices first. This proves the response is already
148+
# flowing and makes the later timeout happen during slice subrequest output.
149+
while ($read < 384 * 1024 && time < $deadline) {
150+
my @ready = $sel->can_read(0.2);
151+
next unless @ready;
152+
153+
my $n = sysread($sock, my $buf, 65536);
154+
last unless $n;
155+
$read += $n;
156+
}
157+
158+
# Without this precondition, the timeout might hit a different request
159+
# state and the regression signal would be ambiguous.
160+
die "stalled HTTP/2 client did not receive enough response bytes: $read"
161+
if $read < 384 * 1024;
162+
163+
# Stop reading for longer than send_timeout (1s) while the connection stays
164+
# open. This is the actual stalled-client trigger.
165+
select undef, undef, undef, 2.5;
166+
close $sock;
167+
}
168+
169+
# Create a multi-slice origin file and warm proxy_cache with a normal client.
170+
# The stalled h2c request below then runs from cached slice subrequests, making
171+
# the timeout path independent of upstream timing.
172+
my $file = "$::HtmlDir/slice.bin";
173+
system("dd", "if=/dev/urandom", "of=$file", "bs=1M", "count=2", "status=none") == 0
174+
or die "failed to create $file";
175+
176+
my $port = $Test::Nginx::Util::ServerPortForClient;
177+
my $cmd = "curl -sS --connect-timeout 5 --max-time 30 "
178+
. "http://127.0.0.1:$port/slice.bin -o /dev/null";
179+
system($cmd) == 0 or die "failed to warm sliced proxy cache: $cmd";
180+
181+
run_stalled_h2_client($port, "/slice.bin");
182+
--- http2
183+
--- request
184+
GET /ping
185+
--- response_body
186+
ok
187+
--- error_log
188+
http slice subrequest
189+
client timed out
190+
--- no_error_log
191+
header already sent
192+
[alert]
193+
[crit]
194+
[emerg]
195+
--- no_shutdown_error_log
196+
header already sent
197+
[alert]
198+
[crit]
199+
[emerg]

‎util/build-with-dd.sh‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ time ngx-build $force $version \
1818
--with-ipv6 \
1919
--with-cc-opt="-DNGX_LUA_USE_ASSERT -I$PCRE2_INC -I$OPENSSL_INC -DDDEBUG=1" \
2020
--with-http_v2_module \
21+
--with-http_slice_module \
2122
--with-http_v3_module \
2223
--with-http_realip_module \
2324
--with-http_ssl_module \

‎util/build-without-ssl.sh‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ time ngx-build $force $version \
1919
--with-ipv6 \
2020
--with-cc-opt="-DNGX_LUA_USE_ASSERT -I$PCRE2_INC" \
2121
--with-http_v2_module \
22+
--with-http_slice_module \
2223
--with-http_realip_module \
2324
--add-module=$root/../ndk-nginx-module \
2425
--add-module=$root/../set-misc-nginx-module \

‎util/build.sh‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ time ngx-build $force $version \
3232
--with-ipv6 \
3333
--with-cc-opt="-DNGX_LUA_USE_ASSERT -I$PCRE2_INC -I$OPENSSL_INC" \
3434
--with-http_v2_module \
35+
--with-http_slice_module \
3536
--with-http_v3_module \
3637
--with-http_realip_module \
3738
--with-http_ssl_module \

0 commit comments

Comments
 (0)