Skip to content

Commit 48d5628

Browse files
bugfix: SIGSEGV in receiveuntil __gc on aborted multipart upload. (openresty#2504)
read_error_retval_handler calls finalize_read_part directly when the receiveuntil iterator's recv errors. That clears u->buf_in but leaves cp->upstream live with cp->state > 0. Later GC fires cleanup_compiled_pattern -> read_prepare, which derefs the now-NULL u->buf_in. Mirror tcp_finalize's cp->upstream = NULL detach so __gc's existing `if (u != NULL)` guard short-circuits. Backtrace: ngx_http_lua_socket_tcp_read_prepare ngx_http_lua_socket_cleanup_compiled_pattern lj_BC_FUNCC gc_call_finalizer gc_finalize gc_onestep lj_gc_fullgc lua_gc lj_cf_collectgarbage lj_BC_FUNCC ngx_http_lua_run_thread ngx_http_lua_socket_tcp_resume_helper ngx_http_lua_access_handler ngx_http_core_access_phase ngx_http_core_run_phases ngx_http_lua_socket_tcp_read ngx_http_request_handler ngx_epoll_process_events ngx_process_events_and_timers ngx_worker_process_cycle ngx_spawn_process ngx_start_worker_processes ngx_master_process_cycle main * add tests. --------- Co-authored-by: lijunlong <lijunlong@openresty.com>
1 parent 41ed26b commit 48d5628

2 files changed

Lines changed: 72 additions & 1 deletion

File tree

src/ngx_http_lua_socket_tcp.c

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4203,6 +4203,12 @@ ngx_http_lua_socket_tcp_finalize_read_part(ngx_http_request_t *r,
42034203
ngx_memzero(&u->buffer, sizeof(ngx_buf_t));
42044204
}
42054205

4206+
/* mirror tcp_finalize: detach cp so its __gc is safe */
4207+
if (u->input_filter_ctx != NULL && u->input_filter_ctx != u) {
4208+
((ngx_http_lua_socket_compiled_pattern_t *)
4209+
u->input_filter_ctx)->upstream = NULL;
4210+
}
4211+
42064212
if (u->raw_downstream || u->body_downstream) {
42074213
if (r->connection->read->timer_set) {
42084214
ngx_del_timer(r->connection->read);

t/067-req-socket.t

Lines changed: 66 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ use Test::Nginx::Socket::Lua $SkipReason ? (skip_all => $SkipReason) : ();
1414

1515
repeat_each(2);
1616

17-
plan tests => repeat_each() * (blocks() * 3 + 9);
17+
plan tests => repeat_each() * (blocks() * 3 + 11);
1818

1919
our $HtmlDir = html_dir;
2020

@@ -1206,3 +1206,68 @@ qr/\Agot the request socket
12061206
\z/ms
12071207
--- no_error_log
12081208
[error]
1209+
1210+
1211+
1212+
=== TEST 20: receiveuntil compiled-pattern GC after aborted multipart body (GH #2503)
1213+
--- config
1214+
location = /t {
1215+
content_by_lua_block {
1216+
local sock = ngx.socket.tcp()
1217+
local ok, err = sock:connect("127.0.0.1", ngx.var.server_port)
1218+
if not ok then
1219+
ngx.say("connect failed: ", err)
1220+
return
1221+
end
1222+
local req = "POST /upload HTTP/1.1\r\n"
1223+
.. "Host: 127.0.0.1\r\n"
1224+
.. "Content-Type: multipart/form-data; "
1225+
.. "boundary=----------GFioQpMK0vv2\r\n"
1226+
.. "Content-Length: 1048576\r\n"
1227+
.. "Connection: close\r\n\r\n"
1228+
.. "------"
1229+
local bytes, err = sock:send(req)
1230+
if not bytes then
1231+
ngx.say("send failed: ", err)
1232+
return
1233+
end
1234+
ngx.sleep(0.2)
1235+
sock:close()
1236+
ngx.sleep(0.5)
1237+
ngx.say("ok")
1238+
}
1239+
}
1240+
1241+
location = /upload {
1242+
content_by_lua_block {
1243+
local BOUNDARY = "----------GFioQpMK0vv2"
1244+
local sock, err = ngx.req.socket()
1245+
if not sock then
1246+
ngx.log(ngx.ERR, "no req socket: ", err)
1247+
return
1248+
end
1249+
sock:settimeout(2000)
1250+
local iter = sock:receiveuntil("--" .. BOUNDARY)
1251+
while true do
1252+
local data, e = iter(1)
1253+
if not data then
1254+
ngx.log(ngx.WARN, "iter err: ", e)
1255+
break
1256+
end
1257+
end
1258+
iter = nil
1259+
collectgarbage("collect")
1260+
collectgarbage("collect")
1261+
ngx.log(ngx.WARN, "receiveuntil gc done, no crash")
1262+
}
1263+
}
1264+
--- request
1265+
GET /t
1266+
--- response_body
1267+
ok
1268+
--- error_log
1269+
receiveuntil gc done, no crash
1270+
--- no_error_log
1271+
[alert]
1272+
[emerg]
1273+
--- skip_eval: 5:defined($ENV{MOCKEAGAIN}) && ($ENV{MOCKEAGAIN} ne "")

0 commit comments

Comments
 (0)