Serialize the HTTP/1.1 statschannel requests

The statschannel truncated test still terminates abruptly sometimes and
it doesn't return the answer for the first query.  This might happen
when the second process_request() discovers there's not enough space
before the sending is complete and the connection is terminated before
the client gets the data.

Change the isc_http, so it pauses the reading when it receives the data
and resumes it only after the sending has completed or there's
incomplete request waiting for more data.

This makes the request processing slightly less efficient, but also less
taxing for the server, because previously all requests that has been
received via single TCP read would be processed in the loop and the
sends would be queued after the read callback has processed a full
buffer.

(cherry picked from commit 13959781cb7dab5cc6cec3e14cf97a9507fc36fe)

diff --git a/lib/isc/httpd.c b/lib/isc/httpd.c
index e9bfbe064a5b328ff741068eb36a021647b86e6b..7760d4e86725e893271cb71671866bf40d4ec88a 100644 (file)
--- a/lib/isc/httpd.c
+++ b/lib/isc/httpd.c
@@ -49,9 +49,10 @@
  * Size the recv buffer to hold at maximum two full buffers from isc_nm_read(),
  * so we don't have to handle the truncation.
  */
-#define HTTP_RECVLEN    ISC_NETMGR_TCP_RECVBUF_SIZE * 2
-#define HTTP_SENDLEN    ISC_NETMGR_TCP_RECVBUF_SIZE
-#define HTTP_HEADERS_NUM 10
+#define HTTP_RECVLEN        ISC_NETMGR_TCP_RECVBUF_SIZE * 2
+#define HTTP_SENDLEN        ISC_NETMGR_TCP_RECVBUF_SIZE
+#define HTTP_HEADERS_NUM     10
+#define HTTP_MAX_REQUEST_LEN 4096
 
 #define HTTPD_CLOSE         0x0001 /* Got a Connection: close header */
 #define HTTPD_FOUNDHOST             0x0002 /* Got a Host: header */
@@ -394,6 +395,10 @@ process_request(isc_httpd_t *httpd, size_t last_len) {
 
        INSIST(pret > 0);
 
+       if (pret > HTTP_MAX_REQUEST_LEN) {
+               return (ISC_R_RANGE);
+       }
+
        httpd->consume = pret;
 
        /*
@@ -745,72 +750,23 @@ httpd_compress(isc_httpd_sendreq_t *req) {
 #endif /* ifdef HAVE_ZLIB */
 
 static void
-httpd_request(isc_nmhandle_t *handle, isc_result_t eresult,
-             isc_region_t *region, void *arg) {
-       isc_result_t result;
-       isc_httpd_t *httpd = NULL;
-       isc_httpdmgr_t *mgr = (isc_httpdmgr_t *)arg;
-       isc_httpdurl_t *url = NULL;
+prepare_response(isc_httpdmgr_t *mgr, isc_httpd_t *httpd,
+                isc_httpd_sendreq_t **reqp) {
+       isc_httpd_sendreq_t *req = NULL;
        isc_time_t now;
-       isc_region_t r;
-       bool is_compressed = false;
        char datebuf[ISC_FORMATHTTPTIMESTAMP_SIZE];
-       size_t limit = 0;
-       size_t last_len;
-
-       httpd = isc_nmhandle_getdata(handle);
+       const char *path = "/";
+       size_t path_len = 1;
+       bool is_compressed = false;
+       isc_httpdurl_t *url = NULL;
+       isc_result_t result;
 
        REQUIRE(VALID_HTTPD(httpd));
-
-       REQUIRE(httpd->handle == handle);
-
-       if (eresult != ISC_R_SUCCESS) {
-               goto cleanup_readhandle;
-       }
-
-       REQUIRE(region != NULL);
-
-       last_len = httpd->recvlen;
-
-       if (httpd->recvlen + region->length > sizeof(httpd->recvbuf)) {
-               goto cleanup_readhandle;
-       }
-
-       /* Store the received data into the recvbuf */
-       REQUIRE(httpd->recvlen + region->length < sizeof(httpd->recvbuf));
-       memmove(httpd->recvbuf + httpd->recvlen, region->base, region->length);
-       httpd->recvlen += region->length;
-
-again:
-       result = process_request(httpd, last_len);
-
-       if (result == ISC_R_NOMORE) {
-               limit = sizeof(httpd->recvbuf) - httpd->recvlen;
-               if (region->length > limit) {
-                       /*
-                        * We need more data, but we don't have space to store
-                        * it
-                        */
-                       goto cleanup_readhandle;
-               }
-
-               memmove(httpd->recvbuf + httpd->recvlen, region->base,
-                       region->length);
-
-               /* Just wait for more data */
-               return;
-       }
-
-       if (result != ISC_R_SUCCESS) {
-               goto cleanup_readhandle;
-       }
+       REQUIRE(reqp != NULL && *reqp == NULL);
 
        isc_time_now(&now);
        isc_time_formathttptimestamp(&now, datebuf, sizeof(datebuf));
 
-       const char *path = "/";
-       size_t path_len = 1;
-
        if (httpd->up.field_set & (1 << ISC_UF_PATH)) {
                path = &httpd->path[httpd->up.field_data[ISC_UF_PATH].off];
                path_len = httpd->up.field_data[ISC_UF_PATH].len;
@@ -826,7 +782,7 @@ again:
        }
        UNLOCK(&mgr->lock);
 
-       isc_httpd_sendreq_t *req = isc__httpd_sendreq_new(httpd);
+       req = isc__httpd_sendreq_new(httpd);
 
        if (url == NULL) {
                result = mgr->render_404(httpd, NULL, NULL, &req->retcode,
@@ -918,7 +874,6 @@ again:
        }
        httpd->recvlen -= httpd->consume;
        httpd->consume = 0;
-       last_len = 0;
 
        /*
         * We don't need to attach to httpd here because it gets only cleaned
@@ -926,29 +881,80 @@ again:
         */
        req->httpd = httpd;
 
+       *reqp = req;
+}
+
+static void
+httpd_request(isc_nmhandle_t *handle, isc_result_t eresult,
+             isc_region_t *region, void *arg) {
+       isc_result_t result;
+       isc_httpdmgr_t *mgr = arg;
+       isc_httpd_t *httpd = NULL;
+       isc_httpd_sendreq_t *req = NULL;
+       isc_region_t r;
+       size_t last_len = 0;
+
+       httpd = isc_nmhandle_getdata(handle);
+
+       REQUIRE(VALID_HTTPD(httpd));
+
+       REQUIRE(httpd->handle == handle);
+       REQUIRE(httpd->readhandle == handle);
+
+       isc_nm_pauseread(httpd->readhandle);
+
+       if (eresult != ISC_R_SUCCESS) {
+               goto close_readhandle;
+       }
+
        /*
-        * Determine total response size.
+        * If we are being called from httpd_senddone(), the last HTTP request
+        * was processed successfully, reset the last_len to 0, even if there's
+        * data in the httpd->recvbuf.
         */
-       isc_buffer_usedregion(req->sendbuffer, &r);
+       last_len = (region == NULL) ? 0 : httpd->recvlen;
 
-       isc_nmhandle_attach(httpd->handle, &req->handle);
-       isc_nm_send(httpd->handle, &r, httpd_senddone, req);
+       /* Store the received data into the recvbuf */
+       if (region != NULL) {
+               if (httpd->recvlen + region->length > sizeof(httpd->recvbuf)) {
+                       goto close_readhandle;
+               }
+
+               memmove(httpd->recvbuf + httpd->recvlen, region->base,
+                       region->length);
+               httpd->recvlen += region->length;
+       }
+
+       result = process_request(httpd, last_len);
+
+       if (result == ISC_R_NOMORE) {
+               if (httpd->recvlen > HTTP_MAX_REQUEST_LEN) {
+                       goto close_readhandle;
+               }
 
-       if ((httpd->flags & HTTPD_CLOSE) != 0) {
-               goto cleanup_readhandle;
+               /* Wait for more data, the readhandle is still attached */
+               isc_nm_resumeread(httpd->readhandle);
+               return;
        }
 
+       if (result != ISC_R_SUCCESS) {
+               goto close_readhandle;
+       }
+
+       prepare_response(mgr, httpd, &req);
+
        /*
-        * The request was successfully completed;
+        * Determine total response size.
         */
-       if (httpd->recvlen > 0) {
-               goto again;
-       }
+       isc_buffer_usedregion(req->sendbuffer, &r);
 
+       isc_nmhandle_attach(httpd->handle, &req->handle);
+       isc_nm_send(httpd->handle, &r, httpd_senddone, req);
+
+       isc_nmhandle_detach(&httpd->readhandle);
        return;
 
-cleanup_readhandle:
-       isc_nm_pauseread(httpd->handle);
+close_readhandle:
        isc_nmhandle_detach(&httpd->readhandle);
 }
 
@@ -1025,15 +1031,19 @@ httpd_addheaderuint(isc_httpd_sendreq_t *req, const char *name, int val) {
 }
 
 static void
-httpd_senddone(isc_nmhandle_t *handle, isc_result_t result, void *arg) {
+httpd_senddone(isc_nmhandle_t *handle, isc_result_t eresult, void *arg) {
        isc_httpd_sendreq_t *req = (isc_httpd_sendreq_t *)arg;
        isc_httpd_t *httpd = req->httpd;
 
        REQUIRE(VALID_HTTPD(httpd));
 
-       if (result != ISC_R_SUCCESS && httpd->readhandle != NULL) {
-               isc_nm_pauseread(httpd->handle);
-               isc_nmhandle_detach(&httpd->readhandle);
+       if (eresult == ISC_R_SUCCESS && (httpd->flags & HTTPD_CLOSE) == 0) {
+               /*
+                * Calling httpd_request() with region NULL restarts
+                * reading.
+                */
+               isc_nmhandle_attach(handle, &httpd->readhandle);
+               httpd_request(handle, ISC_R_SUCCESS, NULL, httpd->mgr);
        }
 
        isc_nmhandle_detach(&handle);