daemon: fix DoH with multiple "parallel" queries in one connection

diff --git a/NEWS b/NEWS
index 1107b88db3b1f4b3ff506cb1d7a33ab4fdd9e976..fd589b89d6e2ca7fcf7c28ceea231f87cb050400 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -3,6 +3,7 @@ Knot Resolver 6.0.12 (2025-0m-dd)
 
 Bugfixes
 --------
+- daemon: fix DoH with multiple "parallel" queries in one connection (#931, !1677)
 - /management/unix-socket: revert to absolute path (#926, !1664)
 - fix `tags` when used in /local-data/rules/*/records (!1670)
 - stats: request latency was very incorrect in some cases (!1676)

diff --git a/daemon/http.c b/daemon/http.c
index 89b5e4c42f8b4f1b34eacfa74fa37bb44f209321..fbfb765185d365d8ec1c9baa0cee85b29587b8aa 100644 (file)
--- a/daemon/http.c
+++ b/daemon/http.c
@@ -72,7 +72,9 @@ struct pl_http_sess_data {
        struct protolayer_data h;
        struct nghttp2_session *h2;
 
-       queue_http_stream streams;  /* Streams present in the wire buffer. */
+       /** Info about streams pending for kr_request creation. */
+       queue_http_stream streams;
+
        trie_t *stream_write_queues;  /* Dictionary of stream data that needs to be freed after write. */
        int32_t incomplete_stream;
        int32_t last_stream;   /* The last used stream - mostly the same as incomplete_stream, but can be used after
@@ -912,12 +914,12 @@ static int pl_http_sess_deinit(struct session2 *session, void *data)
                http_free_headers(stream->headers);
                queue_pop(http->streams);
        }
+       queue_deinit(http->streams);
 
        trie_apply(http->stream_write_queues, stream_write_data_break_err, NULL);
        trie_free(http->stream_write_queues);
 
        http_cleanup_stream(http);
-       queue_deinit(http->streams);
        wire_buf_deinit(&http->wire_buf);
        nghttp2_session_del(http->h2);
 
@@ -989,8 +991,8 @@ static enum protolayer_iter_cb_result pl_http_wrap(
        prov.read_callback = read_callback;
 
        struct pl_http_sess_data *http = sess_data;
-       int32_t stream_id = http->last_stream;
-       int ret = http_send_response(sess_data, stream_id, &prov, HTTP_STATUS_OK);
+       int32_t stream_id = ctx->req->qsource.stream_id;
+       int ret = http_send_response(http, stream_id, &prov, HTTP_STATUS_OK);
        if (ret)
                return protolayer_break(ctx, ret);
 
@@ -1022,10 +1024,11 @@ static void pl_http_request_init(struct session2 *session,
        struct http_stream *stream = &queue_head(http->streams);
        req->qsource.stream_id = stream->id;
        if (stream->headers) {
+               // the request takes ownership of the referred-to memory
                req->qsource.headers = *stream->headers;
                free(stream->headers);
-               stream->headers = NULL;
        }
+       queue_pop(http->streams);
 }
 
 __attribute__((constructor))

diff --git a/daemon/session2.c b/daemon/session2.c
index faca57bfe564a3ce9b40894c470ac2df4a8df700..22c6674a946d0c92cb0ceb17f13f35bcfa054023 100644 (file)
--- a/daemon/session2.c
+++ b/daemon/session2.c
@@ -612,7 +612,7 @@ static int session2_submit(
                struct session2 *session,
                enum protolayer_direction direction, size_t layer_ix,
                struct protolayer_payload payload, const struct comm_info *comm,
-               protolayer_finished_cb cb, void *baton)
+               struct kr_request *req, protolayer_finished_cb cb, void *baton)
 {
        if (session->closing)
                return kr_error(ECANCELED);
@@ -647,6 +647,7 @@ static int session2_submit(
                .direction = direction,
                .layer_ix = layer_ix,
                .session = session,
+               .req = req,
                .finished_cb = cb,
                .finished_cb_baton = baton
        };
@@ -1259,7 +1260,7 @@ int session2_unwrap(struct session2 *s, struct protolayer_payload payload,
                     void *baton)
 {
        return session2_submit(s, PROTOLAYER_UNWRAP,
-                       0, payload, comm, cb, baton);
+                       0, payload, comm, NULL/*req*/, cb, baton);
 }
 
 int session2_unwrap_after(struct session2 *s, enum protolayer_type protocol,
@@ -1272,16 +1273,16 @@ int session2_unwrap_after(struct session2 *s, enum protolayer_type protocol,
        if (kr_fails_assert(ok)) // not found or "last layer"
                return kr_error(EINVAL);
        return session2_submit(s, PROTOLAYER_UNWRAP,
-                       layer_ix + 1, payload, comm, cb, baton);
+                       layer_ix + 1, payload, comm, NULL/*req*/, cb, baton);
 }
 
 int session2_wrap(struct session2 *s, struct protolayer_payload payload,
-                  const struct comm_info *comm, protolayer_finished_cb cb,
-                  void *baton)
+                  const struct comm_info *comm, struct kr_request *req,
+                 protolayer_finished_cb cb, void *baton)
 {
        return session2_submit(s, PROTOLAYER_WRAP,
                        protolayer_grps[s->proto].num_layers - 1,
-                       payload, comm, cb, baton);
+                       payload, comm, req, cb, baton);
 }
 
 int session2_wrap_after(struct session2 *s, enum protolayer_type protocol,
@@ -1293,7 +1294,7 @@ int session2_wrap_after(struct session2 *s, enum protolayer_type protocol,
        if (kr_fails_assert(layer_ix > 0)) // not found or "last layer"
                return kr_error(EINVAL);
        return session2_submit(s, PROTOLAYER_WRAP, layer_ix - 1,
-                       payload, comm, cb, baton);
+                       payload, comm, NULL/*req*/, cb, baton);
 }
 
 static void session2_event_wrap(struct session2 *s, enum protolayer_event_type event, void *baton)
@@ -1644,8 +1645,8 @@ static int session2_transport_pushv(struct session2 *s,
                }
                int ret = session2_wrap(parent,
                                protolayer_payload_iovec(iov, iovcnt, iov_short_lived),
-                               comm, session2_transport_parent_pushv_finished,
-                               ctx);
+                               comm, NULL/*req*/,
+                               session2_transport_parent_pushv_finished, ctx);
                return (ret < 0) ? ret : kr_ok();
 
        default:

diff --git a/daemon/session2.h b/daemon/session2.h
index 5228ad864dfdb6362b41f26c3c81fa76f290e6ca..f94a82fa027a80723b65ad711a33145fe050df73 100644 (file)
--- a/daemon/session2.h
+++ b/daemon/session2.h
@@ -441,6 +441,10 @@ struct protolayer_iter_ctx {
         * automatically - layers may use it to allocate memory. */
        knot_mm_t pool;
 
+       /** Careful!  It's only present sometimes and read-only really.
+        * TODO: maybe a better mechanism should be designed instead of this. */
+       struct kr_request *req;
+
 /* callback for when the layer iteration has ended - read-only for layers: */
        protolayer_finished_cb finished_cb;
        void *finished_cb_baton;
@@ -1087,8 +1091,8 @@ int session2_unwrap_after(struct session2 *s, enum protolayer_type protocol,
  * Returns one of `enum protolayer_ret` or a negative number
  * indicating an error. */
 int session2_wrap(struct session2 *s, struct protolayer_payload payload,
-                  const struct comm_info *comm, protolayer_finished_cb cb,
-                  void *baton);
+                  const struct comm_info *comm, struct kr_request *req,
+                 protolayer_finished_cb cb, void *baton);
 
 /** Same as `session2_wrap`, but looks up the specified `protocol` in the
  * session's assigned protocol group and sends the `payload` to the layer that

diff --git a/daemon/worker.c b/daemon/worker.c
index 07480b24ca6f5e133243971bcada295b8351fe6e..70963afdd09c98613be4a25b2a1c4ee67dc4b7f9 100644 (file)
--- a/daemon/worker.c
+++ b/daemon/worker.c
@@ -667,7 +667,7 @@ static int qr_task_send(struct qr_task *task, struct session2 *session,
        struct protolayer_payload payload = protolayer_payload_buffer(
                        (char *)pkt->wire, pkt->size, false);
        payload.ttl = packet_ttl(pkt);
-       ret = session2_wrap(session, payload, comm, qr_task_wrap_finished, task);
+       ret = session2_wrap(session, payload, comm, &task->ctx->req, qr_task_wrap_finished, task);
 
        if (ret >= 0) {
                session2_touch(session);

diff --git a/lib/resolve.h b/lib/resolve.h
index ef4f3f4678ce10c04a6805ecc04947d64a093339..51d47925adafadb7034b1edeb56843295ba0faba 100644 (file)
--- a/lib/resolve.h
+++ b/lib/resolve.h
@@ -251,7 +251,11 @@ struct kr_request {
                uint32_t price_factor16;
                size_t size; /**< query packet size */
                int32_t stream_id; /**< HTTP/2 stream ID for DoH requests */
-               kr_http_header_array_t headers;  /**< HTTP/2 headers for DoH requests */
+               /** HTTP/2 headers for DoH requests
+                *
+                * Note that this owns malloc-ed memory inside (outside ->pool).
+                */
+               kr_http_header_array_t headers;
        } qsource;
        struct {
                unsigned rtt;                  /**< Current upstream RTT */