fix non-blocking CGI handler and non-blocking SSL-IO

Sun, 11 Jun 2023 15:53:55 +0200

author
Olaf Wintermann <olaf.wintermann@gmail.com>
date
Sun, 11 Jun 2023 15:53:55 +0200
changeset 502
11ac3761c0e3
parent 501
2aa6bd9f166f
child 503
aeaf7db26fac

fix non-blocking CGI handler and non-blocking SSL-IO

src/server/daemon/httplistener.c file | annotate | diff | comparison | revisions
src/server/safs/cgi.c file | annotate | diff | comparison | revisions
src/server/safs/cgi.h file | annotate | diff | comparison | revisions
src/server/util/io.c file | annotate | diff | comparison | revisions
--- 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;

mercurial