ah http1.1 deal with pipelined headers properly

Connections must hold an ah for the whole time they are
processing one header set, even if eg, the headers are
fragmented and it involves network roundtrip times.

However on http1.1 / keepalive, it must drop the ah when
there are no more header sets to deal with, and reacquire
the ah later when more data appears.  It's because the
time between header sets / http1.1 requests is unbounded
and the ah would be tied up forever.

But in the case that we got pipelined http1.1 requests,
even partial already buffered, we must keep the ah,
resetting it instead of dropping it.  Because we store
the rx data conveniently in a per-tsi buffer since it only
does one thing at a time per thread, we cannot go back to
the event loop to await a new ah inside one service action.

But no problem since we definitely already have an ah,
let's just reuse it at http completion time if more rx is
already buffered.

NB: attack.sh makes request with echo | nc, this
accidentally sends a trailing '\n' from the echo showing
this problem.  With this patch attack.sh can complete well.

Signed-off-by: Andy Green <andy.green@linaro.org>

diff --git a/lib/client.c b/lib/client.c
index 844d476..849da5a 100644 (file)
--- a/lib/client.c
+++ b/lib/client.c
@@ -21,7 +21,8 @@
 
 #include "private-libwebsockets.h"
 
-int lws_handshake_client(struct lws *wsi, unsigned char **buf, size_t len)
+int
+lws_handshake_client(struct lws *wsi, unsigned char **buf, size_t len)
 {
        int m;
 
@@ -65,8 +66,9 @@ int lws_handshake_client(struct lws *wsi, unsigned char **buf, size_t len)
        return 0;
 }
 
-int lws_client_socket_service(struct lws_context *context,
-                             struct lws *wsi, struct lws_pollfd *pollfd)
+int
+lws_client_socket_service(struct lws_context *context, struct lws *wsi,
+                         struct lws_pollfd *pollfd)
 {
        struct lws_context_per_thread *pt = &context->pt[(int)wsi->tsi];
        char *p = (char *)&pt->serv_buf[0];

diff --git a/lib/parsers.c b/lib/parsers.c
index bd251c1..a841afc 100644 (file)
--- a/lib/parsers.c
+++ b/lib/parsers.c
@@ -60,9 +60,10 @@ lextable_decode(int pos, char c)
        }
 }
 
-static void
+void
 lws_reset_header_table(struct lws *wsi)
 {
+       lwsl_err("%s: wsi %p\n", __func__, wsi);
        /* init the ah to reflect no headers or data have appeared yet */
        memset(wsi->u.hdr.ah->frag_index, 0, sizeof(wsi->u.hdr.ah->frag_index));
        wsi->u.hdr.ah->nfrag = 0;
@@ -110,7 +111,7 @@ lws_allocate_header_table(struct lws *wsi)
         * weren't able to deliver it right now
         */
        if (pt->ah_count_in_use == context->max_http_header_pool) {
-               lwsl_err("%s: adding %p to ah waiting list\n", __func__, wsi);
+               lwsl_info("%s: adding %p to ah waiting list\n", __func__, wsi);
                wsi->u.hdr.ah_wait_list = pt->ah_wait_list;
                pt->ah_wait_list = wsi;
                pt->ah_wait_list_length++;

diff --git a/lib/private-libwebsockets.h b/lib/private-libwebsockets.h
index cfc0a71..72f71b4 100644 (file)
--- a/lib/private-libwebsockets.h
+++ b/lib/private-libwebsockets.h
@@ -874,11 +874,12 @@ struct _lws_header_related {
        char post_literal_equal;
        unsigned char parser_state; /* enum lws_token_indexes */
        char redirects;
+       char more_rx_waiting;
 };
 
 struct _lws_websocket_related {
        char *rx_ubuf;
-       unsigned int rx_ubuf_alloc;
+       unsigned int rx_ubuf_alloc;
        struct lws *rx_draining_ext_list;
        struct lws *tx_draining_ext_list;
        size_t rx_packet_length;
@@ -1178,6 +1179,9 @@ lws_allocate_header_table(struct lws *wsi);
 LWS_EXTERN int
 lws_free_header_table(struct lws *wsi);
 
+LWS_EXTERN void
+lws_reset_header_table(struct lws *wsi);
+
 LWS_EXTERN char * LWS_WARN_UNUSED_RESULT
 lws_hdr_simple_ptr(struct lws *wsi, enum lws_token_indexes h);
 

diff --git a/lib/server.c b/lib/server.c
index 689d477..62bf7f9 100644 (file)
--- a/lib/server.c
+++ b/lib/server.c
@@ -22,8 +22,9 @@
 
 #include "private-libwebsockets.h"
 
-int lws_context_init_server(struct lws_context_creation_info *info,
-                           struct lws_context *context)
+int
+lws_context_init_server(struct lws_context_creation_info *info,
+                       struct lws_context *context)
 {
 #ifdef LWS_USE_IPV6
        struct sockaddr_in6 serv_addr6;
@@ -181,7 +182,8 @@ _lws_server_listen_accept_flow_control(struct lws *twsi, int on)
        return n;
 }
 
-int lws_http_action(struct lws *wsi)
+int
+lws_http_action(struct lws *wsi)
 {
        enum http_connection_type connection_type;
        enum http_version request_version;
@@ -331,7 +333,8 @@ bail_nuke_ah:
 }
 
 
-int lws_handshake_server(struct lws *wsi, unsigned char **buf, size_t len)
+int
+lws_handshake_server(struct lws *wsi, unsigned char **buf, size_t len)
 {
        struct lws_context *context = lws_get_context(wsi);
        struct allocated_headers *ah;
@@ -343,6 +346,7 @@ int lws_handshake_server(struct lws *wsi, unsigned char **buf, size_t len)
        assert(wsi->u.hdr.ah);
 
        while (len--) {
+               wsi->u.hdr.more_rx_waiting = !!len;
 
                assert(wsi->mode == LWSCM_HTTP_SERVING);
 
@@ -688,15 +692,27 @@ lws_http_transaction_completed(struct lws *wsi)
        wsi->state = LWSS_HTTP;
        wsi->mode = LWSCM_HTTP_SERVING;
        wsi->u.http.content_length = 0;
+       wsi->hdr_parsing_completed = 0;
 
        /* He asked for it to stay alive indefinitely */
        lws_set_timeout(wsi, NO_PENDING_TIMEOUT, 0);
 
-       /* if we still have the headers, drop them and reacquire a new ah when
-        * the new headers arrive.  Otherwise we hog an ah indefinitely,
-        * needlessly.
+       /*
+        * We already know we are on http1.1 / keepalive and the next thing
+        * coming will be another header set.
+        *
+        * If there is no pending rx and we still have the ah, drop it and
+        * reacquire a new ah when the new headers start to arrive.  (Otherwise
+        * we needlessly hog an ah indefinitely.)
+        *
+        * However if there is pending rx and we know from the keepalive state
+        * that is already at least the start of another header set, simply
+        * reset the existing header table and keep it.
         */
-       lws_free_header_table(wsi);
+       if (!wsi->u.hdr.more_rx_waiting)
+               lws_free_header_table(wsi);
+       else
+               lws_reset_header_table(wsi);
 
        /* If we're (re)starting on headers, need other implied init */
        wsi->u.hdr.ues = URIES_IDLE;
@@ -813,10 +829,35 @@ lws_server_socket_service(struct lws_context *context, struct lws *wsi,
                if (!(pollfd->revents & pollfd->events & LWS_POLLIN))
                        goto try_pollout;
 
-               if (wsi->state == LWSS_HTTP && !wsi->u.hdr.ah)
+               if ((wsi->state == LWSS_HTTP ||
+                    wsi->state == LWSS_HTTP_ISSUING_FILE ||
+                    wsi->state == LWSS_HTTP_HEADERS) && !wsi->u.hdr.ah)
                        if (lws_allocate_header_table(wsi))
                                goto try_pollout;
 
+               /*
+                * This is a good situation, the ah allocated and we know there
+                * is header data pending.  However, in http1.1 / keepalive
+                * case, back-to-back header sets pipelined into one packet
+                * is common.
+                *
+                * Ah is defined to be required to stay attached to the wsi
+                * until the current set of header data completes, which may
+                * involve network roundtrips if fragmented.  Typically the
+                * header set is not fragmented and gets done atomically.
+                *
+                * When we complete processing for the first header set, we
+                * normally drop the ah in lws_http_transaction_completed()
+                * since we do not know how long it would be held waiting for
+                * the start of the next header set to arrive.
+                *
+                * However if there is pending data, http1.1 / keepalive mode
+                * is active, we need to retain (just reset) the ah after
+                * dealing with each header set instead of dropping it.
+                *
+                * Otherwise the remaining header data has nowhere to be stored.
+                */
+
                len = lws_ssl_capable_read(wsi, pt->serv_buf,
                                           LWS_MAX_SOCKET_IO_BUF);
                lwsl_debug("%s: wsi %p read %d\r\n", __func__, wsi, len);
@@ -971,10 +1012,9 @@ fail:
  *     the wsi should be left alone.
  */
 
-LWS_VISIBLE int lws_serve_http_file(struct lws *wsi, const char *file,
-                                   const char *content_type,
-                                   const char *other_headers,
-                                   int other_headers_len)
+LWS_VISIBLE int
+lws_serve_http_file(struct lws *wsi, const char *file, const char *content_type,
+                   const char *other_headers, int other_headers_len)
 {
        struct lws_context *context = lws_get_context(wsi);
        struct lws_context_per_thread *pt = &context->pt[(int)wsi->tsi];

diff --git a/test-server/test-server-http.c b/test-server/test-server-http.c
index 7fd421e..76b004f 100644 (file)
--- a/test-server/test-server-http.c
+++ b/test-server/test-server-http.c
@@ -145,7 +145,7 @@ int callback_http(struct lws *wsi, enum lws_callback_reasons reason, void *user,
                        n = 0;
                        while (lws_hdr_copy_fragment(wsi, buf, sizeof(buf),
                                                     WSI_TOKEN_HTTP_URI_ARGS, n) > 0) {
-                               lwsl_info("URI Arg %d: %s\n", ++n, buf);
+                               lwsl_notice("URI Arg %d: %s\n", ++n, buf);
                        }
                }
                if (len < 1) {