Sun, 11 Jun 2023 15:53:55 +0200
fix non-blocking CGI handler and non-blocking SSL-IO
--- a/src/server/daemon/httplistener.c Sat Jun 10 18:12:04 2023 +0200 +++ b/src/server/daemon/httplistener.c Sun Jun 11 15:53:55 2023 +0200 @@ -104,6 +104,8 @@ SSL_CTX_set_options( ctx, SSL_OP_SINGLE_DH_USE | SSL_OP_NO_SSLv3); + SSL_CTX_set_mode(ctx, SSL_MODE_ACCEPT_MOVING_WRITE_BUFFER); + SSL_CTX_set_mode(ctx, SSL_MODE_ENABLE_PARTIAL_WRITE); int error = 0; if(conf->disable_proto.ptr) {
--- a/src/server/safs/cgi.c Sat Jun 10 18:12:04 2023 +0200 +++ b/src/server/safs/cgi.c Sun Jun 11 15:53:55 2023 +0200 @@ -198,6 +198,7 @@ error = 1; } + handler->wait_read = TRUE; handler->events = 2; // 2 events (stdout, stderr) if(error) { @@ -222,28 +223,29 @@ > 0) { handler->writebuf_pos += wr; + handler->count_write += wr; } - if(wr < 0) { - if(errno != EWOULDBLOCK) { + if(handler->writebuf_size - handler->writebuf_pos > 0) { + if(net_errno(sn->csd) != EWOULDBLOCK) { handler->result = REQ_ABORTED; } - handler->wait_write = TRUE; + return 1; - } else { - handler->wait_write = FALSE; } return 0; } static int cgi_try_write(CGIHandler *handler, EventHandler *ev, Session *sn, char *buf, size_t size) { - handler->wait_write = FALSE; + size_t pos = 0; ssize_t wr = 0; while(size - pos > 0 && (wr = net_write(sn->csd, buf + pos, size - pos)) > 0) { pos += wr; + handler->count_write += wr; } - if(wr < 0) { - if(errno == EWOULDBLOCK) { + + if(pos < size) { + if(net_errno(sn->csd) == EWOULDBLOCK) { // copy remaining bytes to the write buffer // we assume there are no remaining bytes in writebuf size_t remaining = size-pos; @@ -268,10 +270,9 @@ handler->events++; handler->poll_out = TRUE; } - handler->wait_write = TRUE; } else { handler->result = REQ_ABORTED; - log_ereport(LOG_FAILURE, "cgi_try_write: %s", strerror(errno)); + log_ereport(LOG_FAILURE, "cgi_try_write: %s", strerror(net_errno(sn->csd))); } return 1; } @@ -282,14 +283,22 @@ int cgi_stdout_readevent(EventHandler *ev, Event *event) { CGIHandler *handler = event->cookie; - return cgi_read_output(handler, ev); + int ret = cgi_read_output(handler, ev); + if(ret == 0) { + handler->wait_read = FALSE; + } + return ret; } int cgi_writeevent(EventHandler *ev, Event *event) { CGIHandler *handler = event->cookie; // cgi_read_output will try to flush the buffer - return cgi_read_output(handler, ev); + int ret = cgi_read_output(handler, ev); + if(ret == 0) { + handler->poll_out = FALSE; + } + return ret; } @@ -303,7 +312,7 @@ // if writebuf is empty, this does nothing and returns 0 if(cgi_try_write_flush(handler, sn)) { if(handler->result == REQ_ABORTED) { - log_ereport(LOG_DEBUG, "cgi-send: req: %p write failed: %s: abort", strerror(errno), rq); + log_ereport(LOG_DEBUG, "cgi-send: req: %p write failed: %s: abort", handler->parser->rq, strerror(net_errno(sn->csd)), rq); return 0; } else { return 1; @@ -375,7 +384,7 @@ if(r < 0 && errno == EWOULDBLOCK) { return 1; } - + handler->cgi_eof = TRUE; return 0; } @@ -473,12 +482,32 @@ Session *sn = parser->sn; Request *rq = parser->rq; + char *event_fn = ""; + if(event->fn == cgi_stdout_readevent) { + event_fn = "stdout"; + } else if(event->fn == cgi_stderr_readevent) { + event_fn = "stderr"; + } else if(event->fn == cgi_writeevent) { + event_fn = "httpout"; + } + log_ereport(LOG_DEBUG, "cgi-send: req: %p finish: event: %d pollout: %d cgi_eof: %d fn: %s", rq, handler->events, handler->poll_out, handler->cgi_eof, event_fn); + if(--handler->events > 0) { - if(handler->events == 1 && handler->poll_out && !handler->wait_write) { - // write event registered, however it will not be activated anymore - // we can safely remove the event - if(event_removepoll(ev, sn->csd)) { - log_ereport(LOG_FAILURE, "cgi_event_finish: event_removepoll: %s", strerror(errno)); + if(handler->events == 1) { + if(handler->poll_out) { + // write event registered, however it will not be activated anymore + // we can safely remove the event + log_ereport(LOG_DEBUG, "cgi-send: req: %p finish: event: 1 remove-poll write", rq); + if(event_removepoll(ev, sn->csd)) { + log_ereport(LOG_FAILURE, "cgi_event_finish: event_removepoll: %s", strerror(errno)); + } + } else if(handler->cgi_eof && handler->wait_read) { + log_ereport(LOG_DEBUG, "cgi-send: req: %p finish: event: 1 remove-poll read", rq); + if(ev_remove_poll(ev, handler->process.out[0])) { + log_ereport(LOG_FAILURE, "cgi_event_finish: ev_remove_poll: %s", strerror(errno)); + } + } else { + return 0; } } else { return 0; @@ -490,7 +519,7 @@ killpg(handler->process.pid, SIGTERM); } - log_ereport(LOG_DEBUG, "cgi-send: req: %p cgi_close", rq); + log_ereport(LOG_DEBUG, "cgi-send: req: %p cgi_close", rq); int exit_code = cgi_close(&handler->process); if(exit_code != 0) { @@ -500,6 +529,7 @@ cgi_parser_free(parser); + WSBool response_length_error = FALSE; // check if content-length set by the cgi script matches the number // of writes, that were written to the stream // this ensures, that broken cgi scripts don't break the connection @@ -512,11 +542,25 @@ LOG_FAILURE, "cgi-send: script: %s: content length mismatch", handler->path); - rq->rq_attr.keep_alive = 0; - handler->result = REQ_ABORTED; + response_length_error = TRUE; } } } + // make sure we haven't lost any bytes + // should not happen unless the non-blocking IO code is buggy + if(handler->result != REQ_ABORTED && handler->parser->response_length != handler->count_write) { + log_ereport( + LOG_FAILURE, + "cgi-send: script: %s: IO error: cgi response length != http response length", + handler->path); + response_length_error = TRUE; + } + + // if the response length is broken, we must close the connection + if(response_length_error) { + rq->rq_attr.keep_alive = 0; + handler->result = REQ_ABORTED; + } net_setnonblock(sn->csd, 0);
--- a/src/server/safs/cgi.h Sat Jun 10 18:12:04 2023 +0200 +++ b/src/server/safs/cgi.h Sun Jun 11 15:53:55 2023 +0200 @@ -104,15 +104,32 @@ size_t writebuf_pos; /* + * number of bytes of the response body sent to the client + * at the end count_write should have the same value as parser->response_length + */ + size_t count_write; + + /* * poll_out event active */ WSBool poll_out; /* + * last read returned EWOULDBLOCK + * waiting for the next read event + */ + WSBool wait_read; + + /* * last write returned EWOULDBLOCK * waiting for the next write event */ - WSBool wait_write; + //WSBool wait_write; + + /* + * cgi pipe (stdout) EOF + */ + WSBool cgi_eof; /* * number of currently open events (stdout, stderr, [stdout])
--- a/src/server/util/io.c Sat Jun 10 18:12:04 2023 +0200 +++ b/src/server/util/io.c Sun Jun 11 15:53:55 2023 +0200 @@ -119,15 +119,21 @@ #ifdef XP_UNIX ssize_t net_sys_write(Sysstream *st, const void *buf, size_t nbytes) { - return write(st->fd, buf, nbytes); + ssize_t r = write(st->fd, buf, nbytes); + st->st.io_errno = errno; + return r; } ssize_t net_sys_writev(Sysstream *st, struct iovec *iovec, int iovcnt) { - return writev(st->fd, iovec, iovcnt); + ssize_t r = writev(st->fd, iovec, iovcnt); + st->st.io_errno = errno; + return r; } ssize_t net_sys_read(Sysstream *st, void *buf, size_t nbytes) { - return read(st->fd, buf, nbytes); + ssize_t r = read(st->fd, buf, nbytes); + st->st.io_errno = errno; + return r; } #ifdef WS_SENDFILE @@ -177,7 +183,7 @@ } else { return net_fallback_sendfile((IOStream*)st, sfd); } - + st->st.io_errno = errno; return ret; } #endif @@ -369,6 +375,7 @@ } ssize_t net_http_write(HttpStream *st, const void *buf, size_t nbytes) { + st->st.io_errno = 0; if(st->write_eof) return 0; IOStream *fd = st->fd; if(!st->chunked_enc) { @@ -442,10 +449,11 @@ ssize_t wv = fd->writev(fd, io, iovec_len); if(wv <= 0) { + st->st.io_errno = net_errno(st->fd); return wv; } - size_t ret_w = 0; + ssize_t ret_w = 0; int i = 0; while(wv > 0) { char *base = io[i].iov_base; @@ -457,6 +465,10 @@ } st->written += ret_w; + if(ret_w == 0) { + st->st.io_errno = EWOULDBLOCK; // not sure if this is really correct + ret_w = -1; + } return ret_w; } } @@ -771,6 +783,12 @@ int ret = SSL_write(st->ssl, buf, nbytes); if(ret <= 0) { st->error = SSL_get_error(st->ssl, ret); + if(st->error == SSL_ERROR_WANT_WRITE || st->error == SSL_ERROR_WANT_READ) { + st->st.io_errno = EWOULDBLOCK; + } else { + st->st.io_errno = -1; + } + ret = -1; } return ret; } @@ -780,12 +798,19 @@ for(int i=0;i<iovcnt;i++) { int ret = SSL_write(st->ssl, iovec[i].iov_base, iovec[i].iov_len); if(ret <= 0) { - st->error = SSL_get_error(st->ssl, ret); - return 0; + if(r == 0) { + st->error = SSL_get_error(st->ssl, ret); + if(st->error == SSL_ERROR_WANT_WRITE || st->error == SSL_ERROR_WANT_READ) { + st->st.io_errno = EWOULDBLOCK; + } else { + st->st.io_errno = -1; + } + } + break; } r += ret; } - return r; + return r == 0 ? -1 : r; } ssize_t net_ssl_read(SSLStream *st, void *buf, size_t nbytes) { @@ -853,7 +878,6 @@ ssize_t net_write(SYS_NETFD fd, const void *buf, size_t nbytes) { ssize_t r = ((IOStream*)fd)->write(fd, buf, nbytes); if(r < 0) { - ((IOStream*)fd)->io_errno = errno; return IO_ERROR; } return r;