diff --git a/src/ngx_http_lua_socket_tcp.c b/src/ngx_http_lua_socket_tcp.c index 3ed71be370..239a6e10b2 100644 --- a/src/ngx_http_lua_socket_tcp.c +++ b/src/ngx_http_lua_socket_tcp.c @@ -5404,6 +5404,34 @@ ngx_http_lua_socket_tcp_setkeepalive(lua_State *L) luaL_checktype(L, 1, LUA_TTABLE); + r = ngx_http_lua_get_req(L); + if (r == NULL) { + return luaL_error(L, "no request found"); + } + + llcf = ngx_http_get_module_loc_conf(r, ngx_http_lua_module); + + /* luaL_checkinteger will throw error if the argument is not a number. + * e.g.: bad argument \#2 to '?' (number expected, got string) + * + * We should check the argument in advance; otherwise, + * throwing an exception in the middle can compromise data integrity. + * e.g.: set pc->connection to NULL without following cleanup. + */ + if (n >= 2 && !lua_isnil(L, 2)) { + timeout = (ngx_msec_t) luaL_checkinteger(L, 2); + + } else { + timeout = llcf->keepalive_timeout; + } + + if (n >= 3 && !lua_isnil(L, 3)) { + pool_size = luaL_checkinteger(L, 3); + + } else { + pool_size = llcf->pool_size; + } + lua_rawgeti(L, 1, SOCKET_CTX_INDEX); u = lua_touserdata(L, -1); lua_pop(L, 1); @@ -5430,11 +5458,6 @@ ngx_http_lua_socket_tcp_setkeepalive(lua_State *L) return 2; } - r = ngx_http_lua_get_req(L); - if (r == NULL) { - return luaL_error(L, "no request found"); - } - if (u->request != r) { return luaL_error(L, "bad request"); } @@ -5505,18 +5528,8 @@ ngx_http_lua_socket_tcp_setkeepalive(lua_State *L) /* stack: obj timeout? size? pools cache_key */ - llcf = ngx_http_get_module_loc_conf(r, ngx_http_lua_module); - if (spool == NULL) { /* create a new socket pool for the current peer key */ - - if (n >= 3 && !lua_isnil(L, 3)) { - pool_size = luaL_checkinteger(L, 3); - - } else { - pool_size = llcf->pool_size; - } - if (pool_size <= 0) { msg = lua_pushfstring(L, "bad \"pool_size\" option value: %d", pool_size); @@ -5580,13 +5593,6 @@ ngx_http_lua_socket_tcp_setkeepalive(lua_State *L) ngx_del_timer(c->write); } - if (n >= 2 && !lua_isnil(L, 2)) { - timeout = (ngx_msec_t) luaL_checkinteger(L, 2); - - } else { - timeout = llcf->keepalive_timeout; - } - #if (NGX_DEBUG) if (timeout == 0) { ngx_log_debug0(NGX_LOG_DEBUG_HTTP, r->connection->log, 0, diff --git a/t/068-socket-keepalive.t b/t/068-socket-keepalive.t index 5ca94e4b72..1660a3a361 100644 --- a/t/068-socket-keepalive.t +++ b/t/068-socket-keepalive.t @@ -3029,3 +3029,163 @@ connected: 1, reused: 0 --- error_log lua tcp socket keepalive create connection pool for key "A" lua tcp socket keepalive create connection pool for key "B" + + + +=== TEST 54: wrong first argument for setkeepalive +--- quic_max_idle_timeout: 1.2 +--- http_config eval + "lua_package_path '$::HtmlDir/?.lua;./?.lua;;';" +--- config + server_tokens off; + location /t { + keepalive_timeout 60s; + + set $port $TEST_NGINX_SERVER_PORT; + content_by_lua_block { + local port = ngx.var.port + + local sock = ngx.socket.tcp() + + local ok, err = sock:connect("127.0.0.1", port) + if not ok then + ngx.say("failed to connect: ", err) + return + end + + ngx.say("connected: ", ok) + + local req = "GET /foo HTTP/1.1\r\nHost: localhost\r\nConnection: keepalive\r\n\r\n" + + local bytes, err = sock:send(req) + if not bytes then + ngx.say("failed to send request: ", err) + return + end + + ngx.say("request sent: ", bytes) + + local reader = sock:receiveuntil("\r\n0\r\n\r\n") + local data, err = reader() + + if not data then + ngx.say("failed to receive response body: ", err) + return + end + + ngx.say("received response of ", #data, " bytes") + + ok, err = sock:setkeepalive() + if not ok then + ngx.say("failed to set reusable: ", err) + end + + ok, err = sock:connect("127.0.0.1", port) + if not ok then + ngx.say("failed to connect: ", err) + return + end + + ok, err = sock:setkeepalive("not a number", "not a number") + if not ok then + ngx.say("failed to set reusable: ", err) + end + } + } + + location /foo { + echo foo; + } + + location /sleep { + echo_sleep 1; + } +--- request +GET /t +--- error_code: +--- response_body +--- error_log eval +qr/\Qbad argument #1 to 'setkeepalive' (number expected, got string)\E/ +--- no_error_log +[crit] +--- timeout: 4 + + + +=== TEST 55: wrong second argument for setkeepalive +--- quic_max_idle_timeout: 1.2 +--- http_config eval + "lua_package_path '$::HtmlDir/?.lua;./?.lua;;';" +--- config + server_tokens off; + location /t { + keepalive_timeout 60s; + + set $port $TEST_NGINX_SERVER_PORT; + content_by_lua_block { + local port = ngx.var.port + + local sock = ngx.socket.tcp() + + local ok, err = sock:connect("127.0.0.1", port) + if not ok then + ngx.say("failed to connect: ", err) + return + end + + ngx.say("connected: ", ok) + + local req = "GET /foo HTTP/1.1\r\nHost: localhost\r\nConnection: keepalive\r\n\r\n" + + local bytes, err = sock:send(req) + if not bytes then + ngx.say("failed to send request: ", err) + return + end + + ngx.say("request sent: ", bytes) + + local reader = sock:receiveuntil("\r\n0\r\n\r\n") + local data, err = reader() + + if not data then + ngx.say("failed to receive response body: ", err) + return + end + + ngx.say("received response of ", #data, " bytes") + + ok, err = sock:setkeepalive() + if not ok then + ngx.say("failed to set reusable: ", err) + end + + ok, err = sock:connect("127.0.0.1", port) + if not ok then + ngx.say("failed to connect: ", err) + return + end + + ok, err = sock:setkeepalive(10, "not a number") + if not ok then + ngx.say("failed to set reusable: ", err) + end + } + } + + location /foo { + echo foo; + } + + location /sleep { + echo_sleep 1; + } +--- request +GET /t +--- error_code: +--- response_body +--- error_log eval +qr/\Qbad argument #2 to 'setkeepalive' (number expected, got string)\E/ +--- no_error_log +[crit] +--- timeout: 4