daemon/http: improve buffer handling

Fixes GET requests and handles some edge cases.

diff --git a/daemon/http.c b/daemon/http.c
index a1202fb13af0ccfeb8c5f5beb4d6cc417adc9348..885cd1a9850c7297cb2ebc32454e234391c63462 100644 (file)
--- a/daemon/http.c
+++ b/daemon/http.c
@@ -47,27 +47,35 @@ static ssize_t send_callback(nghttp2_session *session, const uint8_t *data, size
 
 static int header_callback(nghttp2_session *session, const nghttp2_frame *frame, const uint8_t *name, size_t namelen, const uint8_t *value, size_t valuelen, uint8_t flags, void *user_data)
 {
+       struct http_ctx_t *ctx = (struct http_ctx_t *)user_data;
+       static const char key[] = "dns=";
+
        /* If the HEADERS don't have END_STREAM set, there are some DATA frames,
         * which implies POST method.  Skip header processing for POST. */
-       if (frame->hd.flags & NGHTTP2_FLAG_END_STREAM == 0) {
+       if ((frame->hd.flags & NGHTTP2_FLAG_END_STREAM) == 0) {
                return 0;
        }
 
        /* If there is incomplete data in the buffer, we can't process the new stream. */
        if (ctx->incomplete_stream) {
                kr_log_verbose("[http] refusing new http stream due to incomplete data from other stream\n");
-               nghttp2_submit_rst_stream(session, NGHTTP2_FLAG_NONE, stream_id, NGHTTP2_REFUSED_STREAM);
+               nghttp2_submit_rst_stream(session, NGHTTP2_FLAG_NONE, frame->hd.stream_id, NGHTTP2_REFUSED_STREAM);
                return 0;
        }
 
-       static const char key[] = "dns=";
-       struct http_ctx_t *ctx = (struct http_ctx_t *)user_data;
        if (!strcasecmp(":path", (const char *)name)) {
                char *beg = strstr((const char *)value, key);
                if (beg) {
                        beg += sizeof(key) - 1;
                        char *end = strchrnul(beg, '&');
-                       ctx->wire_len = kr_base64url_decode((uint8_t*)beg, end - beg, ctx->wire + sizeof(uint16_t), ctx->wire_len - sizeof(uint16_t));
+                       ctx->buf_pos = sizeof(uint16_t);
+                       ssize_t remaining = ctx->buf_size - ctx->submitted - ctx->buf_pos;
+                       ssize_t ret = kr_base64url_decode((uint8_t*)beg, end - beg, ctx->buf + ctx->buf_pos, remaining);
+                       if (ret < 0) {
+                               nghttp2_submit_rst_stream(session, NGHTTP2_FLAG_NONE, frame->hd.stream_id, NGHTTP2_REFUSED_STREAM);
+                               return 0;  // TODO maybe return NGHTTP2_ERR_TEMPORAL_CALLBACK_FAILURE?
+                       }
+                       ctx->buf_pos += ret;
                        queue_push(ctx->streams, frame->hd.stream_id);
                }
        }
@@ -87,16 +95,22 @@ static int data_chunk_recv_callback(nghttp2_session *session, uint8_t flags, int
                return 0;
        }
 
-       // TODO is there enough space in the wire buffer?
+       /* Check message and its length can still fit into the wire buffer. */
+       ssize_t remaining = ctx->buf_size - ctx->submitted - ctx->buf_pos;
+       if ((len + 2) > remaining) {
+               kr_log_verbose("[http] insufficient space in buffer: remaining %zd B, required %zd B\n", remaining, len + 2);
+               return NGHTTP2_ERR_CALLBACK_FAILURE;
+       }
+
        if (!ctx->incomplete_stream) {
                ctx->incomplete_stream = true;
                queue_push(ctx->streams, stream_id);
 
-               ctx->wire += sizeof(uint16_t);
-               ctx->wire_len = 0;
+               /* 2B at the start of buffer is reserved for message length. */
+               ctx->buf_pos = sizeof(uint16_t);
        }
-       memcpy(ctx->wire + ctx->wire_len, data, len);
-       ctx->wire_len += len;
+       memcpy(ctx->buf + ctx->buf_pos, data, len);
+       ctx->buf_pos += len;
 
        return 0;
 }
@@ -105,12 +119,19 @@ static int on_frame_recv_callback(nghttp2_session *session, const nghttp2_frame
 {
        struct http_ctx_t *ctx = (struct http_ctx_t *)user_data;
 
-       if (frame->hd.flags & NGHTTP2_FLAG_END_STREAM && ctx->incomplete_stream) {
+       if ((frame->hd.flags & NGHTTP2_FLAG_END_STREAM) && ctx->buf_pos != 0) {
                ctx->incomplete_stream = false;
 
-               knot_wire_write_u16(ctx->wire - sizeof(uint16_t), ctx->wire_len);  // TODO wire_len can be overflow when negative  int32_t
-               ctx->submitted += ctx->wire_len + sizeof(uint16_t);
-               ctx->wire += ctx->wire_len;
+               ssize_t len = ctx->buf_pos - sizeof(uint16_t);
+               if (len <= 0 || len > KNOT_WIRE_MAX_PKTSIZE) {
+                       kr_log_verbose("[http] invalid dnsmsg size: %zd B\n", len);
+                       return NGHTTP2_ERR_CALLBACK_FAILURE;
+               }
+
+               knot_wire_write_u16(ctx->buf, len);
+               ctx->submitted += ctx->buf_pos;
+               ctx->buf += ctx->buf_pos;
+               ctx->buf_pos = 0;
        }
 
        return 0;
@@ -154,9 +175,10 @@ ssize_t http_process_input_data(struct session *s, const uint8_t *in_buf, ssize_
        }
 
        http_p->submitted = 0;
-       http_p->wire_start_idx = session_wirebuf_get_free_start(s);
-       http_p->wire = http_p->wire_start_idx;
-       // http_p->wire_len = session_wirebuf_get_free_size(s);  // TODO initialize this for GET
+       http_p->buf = session_wirebuf_get_free_start(s);
+       http_p->buf_pos = 0;
+       http_p->buf_size = session_wirebuf_get_free_size(s);
+
        ssize_t ret = 0;
        if ((ret = nghttp2_session_mem_recv(http_p->session, in_buf, in_buf_len)) < 0) {
                kr_log_error("[http] nghttp2_session_mem_recv failed: %s (%zd)\n", nghttp2_strerror(ret), ret);

diff --git a/daemon/http.h b/daemon/http.h
index 767c3d889dc3046fb78aa72303efed99124744ad..87c89463aa6275ae468df0e2cbba334daa24f84d 100644 (file)
--- a/daemon/http.h
+++ b/daemon/http.h
@@ -28,9 +28,9 @@ struct http_ctx_t {
        queue_int32_t streams;  /* List of stream IDs of read HTTP/2 frames. */
        bool incomplete_stream;
        ssize_t submitted;
-       uint8_t *wire;
-       uint8_t *wire_start_idx;
-       int32_t wire_len;
+       uint8_t *buf;  /* Part of the session->wire_buf that belongs to current HTTP/2 stream. */
+       ssize_t buf_pos;
+       ssize_t buf_size;
 };
 
 struct http_ctx_t* http_new(http_send_callback cb, void *user_ctx);

diff --git a/daemon/session.c b/daemon/session.c
index 48ceafeae631a28eaff5d0986c3f4f7e858f9ebd..92890eb8371f93c74c5cb4938b308767510b4e3e 100644 (file)
--- a/daemon/session.c
+++ b/daemon/session.c
@@ -17,6 +17,9 @@
 
 #define TLS_CHUNK_SIZE (16 * 1024)
 
+/* Initial max frame size: https://tools.ietf.org/html/rfc7540#section-6.5.2 */
+#define HTTP_MAX_FRAME_SIZE 16384
+
 /* Per-socket (TCP or UDP) persistent structure.
  *
  * In particular note that for UDP clients it's just one session (per socket)
@@ -329,6 +332,9 @@ struct session *session_new(uv_handle_t *handle, bool has_tls, bool has_http)
                        session->sflags.has_tls = true;
                }
                if (has_http) {
+                       /* When decoding large packets,
+                        * HTTP/2 frames can be up to 16 KB by default. */
+                       wire_buffer_size += HTTP_MAX_FRAME_SIZE;
                        session->sflags.has_http = true;
                }
                uint8_t *wire_buf = malloc(wire_buffer_size);