From db8f091b32e55f73a89aed2ab98ce0ef4792db8c Mon Sep 17 00:00:00 2001 From: hyung-hwan Date: Thu, 16 Nov 2023 19:20:50 +0900 Subject: [PATCH] fixed a wrong cleanup bug in lib/pro.c fixed the bugs in various http request handlers --- lib/http-cgi.c | 30 +++++++++++++++++++++++------- lib/http-fcgi.c | 14 +++++++++++++- lib/http-file.c | 12 +++++++++--- lib/http-prxy.c | 14 +++++++++++++- lib/http-svr.c | 13 ++++++++++--- lib/http-thr.c | 14 +++++++++++++- lib/http-txt.c | 12 +++++++++++- lib/pro.c | 2 +- 8 files changed, 93 insertions(+), 18 deletions(-) diff --git a/lib/http-cgi.c b/lib/http-cgi.c index 47bbfe0..256c900 100644 --- a/lib/http-cgi.c +++ b/lib/http-cgi.c @@ -31,6 +31,7 @@ #include #include #include /* setenv, clearenv */ +#include #if defined(HAVE_CRT_EXTERNS_H) # include /* _NSGetEnviron */ @@ -116,7 +117,7 @@ static HIO_INLINE void cgi_mark_over (cgi_t* cgi, int over_bits) old_over = cgi->over; cgi->over |= over_bits; - HIO_DEBUG8 (hio, "HTTS(%p) - cgi(t=%p,c=%p[%d],p=%p) - old_over=%x | new-bits=%x => over=%x\n", cgi->htts, cgi, cgi->task_client, (cgi->task_csck? cgi->task_csck->hnd: -1), cgi->peer, (int)old_over, (int)over_bits, (int)cgi->over); + HIO_DEBUG8 (hio, "HTTS(%p) - cgi(t=%p,c=%p[%d],p=%p) - old_over=%x | new-bits=%x => over=%x\n", cgi->htts, cgi, cgi->task_client, (cgi->task_csck? cgi->task_csck->hnd: -1), cgi->peer, (int)old_over, (int)over_bits, (int)cgi->over); if (!(old_over & CGI_OVER_READ_FROM_CLIENT) && (cgi->over & CGI_OVER_READ_FROM_CLIENT)) { @@ -206,8 +207,11 @@ static void cgi_peer_on_close (hio_dev_pro_t* pro, hio_dev_pro_sid_t sid) { case HIO_DEV_PRO_MASTER: HIO_DEBUG3 (hio, "HTTS(%p) - peer %p(pid=%d) closing master\n", cgi->htts, pro, (int)pro->child_pid); + /* reset cgi->peer before calling unbind_task_from_peer() because this is the peer close callback */ cgi->peer = HIO_NULL; + HIO_SVC_HTTS_TASK_RCDOWN((hio_svc_htts_task_t*)cgi); /* if not reset, this would be done in unbind_task_from_peer */ + unbind_task_from_peer (cgi, 1); break; @@ -373,8 +377,8 @@ static int peer_htrd_peek (hio_htrd_t* htrd, hio_htre_t* req) chunked = cgi->task_keep_client_alive && !req->attr.content_length; if (hio_svc_htts_task_startreshdr(cgi, status_code, status_desc, chunked) <= -1 || - hio_htre_walkheaders(req, peer_capture_response_header, cgi) <= -1 || - hio_svc_htts_task_endreshdr(cgi) <= -1) return -1; + hio_htre_walkheaders(req, peer_capture_response_header, cgi) <= -1 || + hio_svc_htts_task_endreshdr(cgi) <= -1) return -1; } return 0; @@ -817,6 +821,7 @@ static int bind_task_to_peer (cgi_t* cgi, hio_dev_sck_t* csck, hio_htre_t* req, if (access(mi.cmd, X_OK) == -1) { /* not executable */ + hio_seterrwithsyserr (hio, 0, errno); hio_freemem (hio, fc.actual_script); return -2; } @@ -922,7 +927,7 @@ int hio_svc_htts_docgi (hio_svc_htts_t* htts, hio_dev_sck_t* csck, hio_htre_t* r hio_t* hio = htts->hio; hio_svc_htts_cli_t* cli = hio_dev_sck_getxtn(csck); cgi_t* cgi = HIO_NULL; - int n; + int n, bound_to_client = 0, bound_to_peer = 0; /* ensure that you call this function before any contents is received */ HIO_ASSERT (hio, hio_htre_getcontentlen(req) == 0); @@ -930,16 +935,20 @@ int hio_svc_htts_docgi (hio_svc_htts_t* htts, hio_dev_sck_t* csck, hio_htre_t* r cgi = (cgi_t*)hio_svc_htts_task_make(htts, HIO_SIZEOF(*cgi), cgi_on_kill, req, csck); if (HIO_UNLIKELY(!cgi)) goto oops; + HIO_SVC_HTTS_TASK_RCUP((hio_svc_htts_task_t*)cgi); cgi->on_kill = on_kill; cgi->options = options; bind_task_to_client (cgi, csck); + bound_to_client = 1; + if ((n = bind_task_to_peer(cgi, csck, req, docroot, script)) <= -1) { hio_svc_htts_task_sendfinalres(cgi, (n == 2? HIO_HTTP_STATUS_FORBIDDEN: HIO_HTTP_STATUS_INTERNAL_SERVER_ERROR), HIO_NULL, HIO_NULL, 1); - goto oops; /* TODO: must not go to oops. just destroy the cgi and finalize the request .. */ + goto oops; } + bound_to_peer = 1; if (hio_svc_htts_task_handleexpect100(cgi) <= -1) goto oops; if (setup_for_content_length(cgi, req) <= -1) goto oops; @@ -948,10 +957,17 @@ int hio_svc_htts_docgi (hio_svc_htts_t* htts, hio_dev_sck_t* csck, hio_htre_t* r if (hio_dev_sck_read(csck, !(cgi->over & CGI_OVER_READ_FROM_CLIENT)) <= -1) goto oops; HIO_SVC_HTTS_TASKL_APPEND_TASK (&htts->task, (hio_svc_htts_task_t*)cgi); + HIO_SVC_HTTS_TASK_RCDOWN((hio_svc_htts_task_t*)cgi); return 0; oops: - HIO_DEBUG2 (hio, "HTTS(%p) - FAILURE in docgi - socket(%p)\n", htts, csck); - if (cgi) cgi_halt_participating_devices (cgi); + HIO_DEBUG3 (hio, "HTTS(%p) - FAILURE in docgi - socket(%p) - %js\n", htts, csck, hio_geterrmsg(hio)); + if (cgi) + { + if (bound_to_peer) unbind_task_from_peer (cgi, 1); + if (bound_to_client) unbind_task_from_client (cgi, 1); + cgi_halt_participating_devices (cgi); + HIO_SVC_HTTS_TASK_RCDOWN((hio_svc_htts_task_t*)cgi); + } return -1; } diff --git a/lib/http-fcgi.c b/lib/http-fcgi.c index b78c3f9..ba19520 100644 --- a/lib/http-fcgi.c +++ b/lib/http-fcgi.c @@ -752,6 +752,7 @@ int hio_svc_htts_dofcgi (hio_svc_htts_t* htts, hio_dev_sck_t* csck, hio_htre_t* hio_t* hio = htts->hio; hio_svc_htts_cli_t* cli = hio_dev_sck_getxtn(csck); fcgi_t* fcgi = HIO_NULL; + int bound_to_client = 0, bound_to_peer = 0; /* ensure that you call this function before any contents is received */ HIO_ASSERT (hio, hio_htre_getcontentlen(req) == 0); @@ -764,11 +765,15 @@ int hio_svc_htts_dofcgi (hio_svc_htts_t* htts, hio_dev_sck_t* csck, hio_htre_t* fcgi = (fcgi_t*)hio_svc_htts_task_make(htts, HIO_SIZEOF(*fcgi), fcgi_on_kill, req, csck); if (HIO_UNLIKELY(!fcgi)) goto oops; + HIO_SVC_HTTS_TASK_RCUP ((hio_svc_htts_task_t*)fcgi); fcgi->on_kill = on_kill; /* custom on_kill handler by the caller */ bind_task_to_client (fcgi, csck); + bound_to_client = 1; + if (bind_task_to_peer(fcgi, fcgis_addr) <= -1) goto oops; + bound_to_peer = 1; if (hio_svc_htts_task_handleexpect100(fcgi) <= -1) goto oops; if (setup_for_content_length(fcgi, req) <= -1) goto oops; @@ -783,10 +788,17 @@ int hio_svc_htts_dofcgi (hio_svc_htts_t* htts, hio_dev_sck_t* csck, hio_htre_t* if (hio_svc_fcgic_writeparam(fcgi->peer, HIO_NULL, 0, HIO_NULL, 0) <= -1) goto oops; /* end of params */ HIO_SVC_HTTS_TASKL_APPEND_TASK (&htts->task, (hio_svc_htts_task_t*)fcgi); + HIO_SVC_HTTS_TASK_RCDOWN ((hio_svc_htts_task_t*)fcgi); return 0; oops: HIO_DEBUG2 (hio, "HTTS(%p) - FAILURE in dofcgi - socket(%p)\n", htts, csck); - if (fcgi) fcgi_halt_participating_devices (fcgi); + if (fcgi) + { + if (bound_to_peer) unbind_task_from_peer (fcgi, 1); + if (bound_to_client) unbind_task_from_client (fcgi, 1); + fcgi_halt_participating_devices ((hio_svc_htts_task_t*)fcgi); + HIO_SVC_HTTS_TASK_RCDOWN ((hio_svc_htts_task_t*)fcgi); + } return -1; } diff --git a/lib/http-file.c b/lib/http-file.c index fbda2f2..2643020 100644 --- a/lib/http-file.c +++ b/lib/http-file.c @@ -873,6 +873,7 @@ int hio_svc_htts_dofile (hio_svc_htts_t* htts, hio_dev_sck_t* csck, hio_htre_t* hio_svc_htts_cli_t* cli = hio_dev_sck_getxtn(csck); file_t* file = HIO_NULL; hio_bch_t* actual_file = HIO_NULL; + int bound_to_client = 0, bound_to_peer = 0; /* ensure that you call this function before any contents is received */ HIO_ASSERT (hio, hio_htre_getcontentlen(req) == 0); @@ -885,7 +886,7 @@ int hio_svc_htts_dofile (hio_svc_htts_t* htts, hio_dev_sck_t* csck, hio_htre_t* file = (file_t*)hio_svc_htts_task_make(htts, HIO_SIZEOF(*file), file_on_kill, req, csck); if (HIO_UNLIKELY(!file)) goto oops; - HIO_SVC_HTTS_TASK_RCUP (file); /* for temporary protection */ + HIO_SVC_HTTS_TASK_RCUP ((hio_svc_htts_task_t*)file); /* for temporary protection */ file->on_kill = on_kill; file->options = options; @@ -895,26 +896,31 @@ int hio_svc_htts_dofile (hio_svc_htts_t* htts, hio_dev_sck_t* csck, hio_htre_t* file->peer = -1; bind_task_to_client (file, csck); /* the file task's reference count is incremented */ + bound_to_client = 1; if (hio_svc_htts_task_handleexpect100(file) <= -1) goto oops; if (setup_for_content_length(file, req) <= -1) goto oops; if (bind_task_to_peer(file, req, actual_file, mime_type) <= -1) goto oops; + bound_to_peer = 1; /* TODO: store current input watching state and use it when destroying the file data */ if (hio_dev_sck_read(csck, !(file->over & FILE_OVER_READ_FROM_CLIENT)) <= -1) goto oops; hio_freemem (hio, actual_file); HIO_SVC_HTTS_TASKL_APPEND_TASK (&htts->task, (hio_svc_htts_task_t*)file); - HIO_SVC_HTTS_TASK_RCDOWN (file); + HIO_SVC_HTTS_TASK_RCDOWN ((hio_svc_htts_task_t*)file); return 0; oops: HIO_DEBUG2 (hio, "HTTS(%p) - file(c=%d) failure\n", htts, csck->hnd); if (file) { + if (bound_to_peer) unbind_task_from_peer (file, 0); + if (bound_to_client) unbind_task_from_client (file, 0); + file_halt_participating_devices (file); - HIO_SVC_HTTS_TASK_RCDOWN (file); + HIO_SVC_HTTS_TASK_RCDOWN ((hio_svc_htts_task_t*)file); } if (actual_file) hio_freemem (hio, actual_file); return -1; diff --git a/lib/http-prxy.c b/lib/http-prxy.c index 42ccf68..21ec762 100644 --- a/lib/http-prxy.c +++ b/lib/http-prxy.c @@ -919,6 +919,7 @@ int hio_svc_htts_doprxy (hio_svc_htts_t* htts, hio_dev_sck_t* csck, hio_htre_t* hio_t* hio = htts->hio; hio_svc_htts_cli_t* cli = hio_dev_sck_getxtn(csck); prxy_t* prxy = HIO_NULL; + int bound_to_client = 0, bound_to_peer = 0; int n; /* ensure that you call this function before any contents is received */ @@ -927,16 +928,20 @@ int hio_svc_htts_doprxy (hio_svc_htts_t* htts, hio_dev_sck_t* csck, hio_htre_t* prxy = (prxy_t*)hio_svc_htts_task_make(htts, HIO_SIZEOF(*prxy), prxy_on_kill, req, csck); if (HIO_UNLIKELY(!prxy)) goto oops; + HIO_SVC_HTTS_TASK_RCUP ((hio_svc_htts_task_t*)prxy); prxy->on_kill = on_kill; prxy->options = options; bind_task_to_client (prxy, csck); + bound_to_client = 1; + if ((n = bind_task_to_peer(prxy, csck, req, tgt_addr)) <= -1) { hio_svc_htts_task_sendfinalres(prxy, (n == 2? HIO_HTTP_STATUS_FORBIDDEN: HIO_HTTP_STATUS_INTERNAL_SERVER_ERROR), HIO_NULL, HIO_NULL, 1); goto oops; /* TODO: must not go to oops. just destroy the prxy and finalize the request .. */ } + bound_to_peer = 1; if (hio_svc_htts_task_handleexpect100(prxy) <= -1) goto oops; if (setup_for_content_length(prxy, req) <= -1) goto oops; @@ -945,10 +950,17 @@ int hio_svc_htts_doprxy (hio_svc_htts_t* htts, hio_dev_sck_t* csck, hio_htre_t* if (hio_dev_sck_read(csck, !(prxy->over & PRXY_OVER_READ_FROM_CLIENT)) <= -1) goto oops; HIO_SVC_HTTS_TASKL_APPEND_TASK (&htts->task, (hio_svc_htts_task_t*)prxy); + HIO_SVC_HTTS_TASK_RCDOWN ((hio_svc_htts_task_t*)prxy); return 0; oops: HIO_DEBUG2 (hio, "HTTS(%p) - FAILURE in doprxy - socket(%p)\n", htts, csck); - if (prxy) prxy_halt_participating_devices (prxy); + if (prxy) + { + if (bound_to_peer) unbind_task_from_peer (prxy, 1); + if (bound_to_client) unbind_task_from_client (prxy, 1); + prxy_halt_participating_devices (prxy); + HIO_SVC_HTTS_TASK_RCDOWN ((hio_svc_htts_task_t*)prxy); + } return -1; } diff --git a/lib/http-svr.c b/lib/http-svr.c index a1ad812..3566c47 100644 --- a/lib/http-svr.c +++ b/lib/http-svr.c @@ -335,6 +335,10 @@ static int client_on_write (hio_dev_sck_t* sck, hio_iolen_t wrlen, void* wrctx, hio_svc_htts_t* htts = cli->htts; hio_svc_htts_task_t* task = cli->task; + /* the callback may get called after the client structure has been delinked from the task structure. + * in this case, `task` points to HIO_NULL. this can happen because the task doesn't wait until + * this write callback has been invoked terminates the job after having sent some reply data */ + HIO_ASSERT (hio, cli->l_idx == INVALID_LIDX); /* handle event if it's write by self */ @@ -348,12 +352,15 @@ static int client_on_write (hio_dev_sck_t* sck, hio_iolen_t wrlen, void* wrctx, else if (wrlen == 0) { /* if connect: is keep-alive, this part may not be called */ - task->task_res_pending_writes--; + if (task) task->task_res_pending_writes--; } else { - HIO_ASSERT (hio, task->task_res_pending_writes > 0); - task->task_res_pending_writes--; + if (task) + { + HIO_ASSERT (hio, task->task_res_pending_writes > 0); + task->task_res_pending_writes--; + } } } diff --git a/lib/http-thr.c b/lib/http-thr.c index 20a0681..1de0210 100644 --- a/lib/http-thr.c +++ b/lib/http-thr.c @@ -839,18 +839,23 @@ int hio_svc_htts_dothr (hio_svc_htts_t* htts, hio_dev_sck_t* csck, hio_htre_t* r hio_t* hio = htts->hio; hio_svc_htts_cli_t* cli = hio_dev_sck_getxtn(csck); thr_t* thr = HIO_NULL; + int bound_to_client = 0, bound_to_peer = 0; /* ensure that you call this function before any contents is received */ HIO_ASSERT (hio, hio_htre_getcontentlen(req) == 0); thr = (thr_t*)hio_svc_htts_task_make(htts, HIO_SIZEOF(*thr), thr_on_kill, req, csck); if (HIO_UNLIKELY(!thr)) goto oops; + HIO_SVC_HTTS_TASK_RCUP ((hio_svc_htts_task_t*)thr); thr->on_kill = on_kill; thr->options = options; bind_task_to_client (thr, csck); + bound_to_client = 1; + if (bind_task_to_peer(thr, csck, req, func, ctx) <= -1) goto oops; + bound_to_peer = 1; if (hio_svc_htts_task_handleexpect100(thr) <= -1) goto oops; if (setup_for_content_length(thr, req) <= -1) goto oops; @@ -859,10 +864,17 @@ int hio_svc_htts_dothr (hio_svc_htts_t* htts, hio_dev_sck_t* csck, hio_htre_t* r if (hio_dev_sck_read(csck, !(thr->over & THR_OVER_READ_FROM_CLIENT)) <= -1) goto oops; HIO_SVC_HTTS_TASKL_APPEND_TASK (&htts->task, (hio_svc_htts_task_t*)thr); + HIO_SVC_HTTS_TASK_RCDOWN ((hio_svc_htts_task_t*)thr); return 0; oops: HIO_DEBUG2 (hio, "HTTS(%p) - FAILURE in dothr - socket(%p)\n", htts, csck); - if (thr) thr_halt_participating_devices (thr); + if (thr) + { + if (bound_to_peer) unbind_task_from_peer (thr, 1); + if (bound_to_client) unbind_task_from_client (thr, 1); + thr_halt_participating_devices (thr); + HIO_SVC_HTTS_TASK_RCDOWN ((hio_svc_htts_task_t*)thr); + } return -1; } diff --git a/lib/http-txt.c b/lib/http-txt.c index 83cee94..2f49277 100644 --- a/lib/http-txt.c +++ b/lib/http-txt.c @@ -331,6 +331,7 @@ int hio_svc_htts_dotxt (hio_svc_htts_t* htts, hio_dev_sck_t* csck, hio_htre_t* r hio_t* hio = htts->hio; hio_svc_htts_cli_t* cli = hio_dev_sck_getxtn(csck); txt_t* txt = HIO_NULL; + int bound_to_client = 0; /* ensure that you call this function before any contents is received */ HIO_ASSERT (hio, hio_htre_getcontentlen(req) == 0); @@ -338,11 +339,14 @@ int hio_svc_htts_dotxt (hio_svc_htts_t* htts, hio_dev_sck_t* csck, hio_htre_t* r txt = (txt_t*)hio_svc_htts_task_make(htts, HIO_SIZEOF(*txt), txt_on_kill, req, csck); if (HIO_UNLIKELY(!txt)) goto oops; + HIO_SVC_HTTS_TASK_RCUP ((hio_svc_htts_task_t*)txt); + txt->on_kill = on_kill; txt->options = options; bind_task_to_client (txt, csck); + bound_to_client = 1; if (req->flags & HIO_HTRE_ATTR_EXPECT100) { @@ -363,10 +367,16 @@ int hio_svc_htts_dotxt (hio_svc_htts_t* htts, hio_dev_sck_t* csck, hio_htre_t* r if (hio_svc_htts_task_sendfinalres(txt, HIO_HTTP_STATUS_OK, content_type, content_text, 0) <= -1) goto oops; HIO_SVC_HTTS_TASKL_APPEND_TASK (&htts->task, (hio_svc_htts_task_t*)txt); + HIO_SVC_HTTS_TASK_RCDOWN ((hio_svc_htts_task_t*)txt); return 0; oops: HIO_DEBUG2 (hio, "HTTS(%p) - FAILURE in dotxt - socket(%p)\n", htts, csck); - if (txt) txt_halt_participating_devices (txt); + if (txt) + { + if (bound_to_client) unbind_task_from_client (txt, 1); + txt_halt_participating_devices (txt); + HIO_SVC_HTTS_TASK_RCDOWN ((hio_svc_htts_task_t*)txt); + } return -1; } diff --git a/lib/pro.c b/lib/pro.c index 6864f8a..92e36ad 100644 --- a/lib/pro.c +++ b/lib/pro.c @@ -408,7 +408,7 @@ static int dev_pro_make_master (hio_dev_t* dev, void* ctx) return 0; oops: - for (i = minidx; i < maxidx; i++) + for (i = minidx; i <= maxidx; i++) { if (pfds[i] != HIO_SYSHND_INVALID) close (pfds[i]); }