From 7f367b0c7fa9b61b1873d0b4655d56b0ffdd17b7 Mon Sep 17 00:00:00 2001
From: Evan Hunt <each@isc.org>
Date: Wed, 21 Apr 2021 17:35:07 -0700
Subject: [PATCH] use the correct handle when calling the read callback

when calling isc_nm_read() on an HTTP socket, the read callback
was being run with the incorrect handle. this has been corrected.
---
 lib/isc/netmgr/http.c    | 33 ++++++++++++++++++++++++++-------
 lib/isc/tests/doh_test.c | 13 +++----------
 2 files changed, 29 insertions(+), 17 deletions(-)

diff --git a/lib/isc/netmgr/http.c b/lib/isc/netmgr/http.c
index 64fbc39ab1..a77216000d 100644
--- a/lib/isc/netmgr/http.c
+++ b/lib/isc/netmgr/http.c
@@ -122,6 +122,7 @@ struct isc_nm_http_session {
 	size_t nsstreams;
 
 	isc_nmhandle_t *handle;
+	isc_nmhandle_t *client_httphandle;
 	isc_nmsocket_t *serversocket;
 	isc_nmiface_t server_iface;
 
@@ -412,11 +413,13 @@ finish_http_session(isc_nm_http_session_t *session) {
 	if (session->closed) {
 		return;
 	}
+
 	if (session->handle != NULL) {
 		if (!session->closed) {
 			session->closed = true;
 			isc_nm_cancelread(session->handle);
 		}
+
 		if (!ISC_LIST_EMPTY(session->cstreams)) {
 			http_cstream_t *cstream =
 				ISC_LIST_HEAD(session->cstreams);
@@ -426,7 +429,8 @@ finish_http_session(isc_nm_http_session_t *session) {
 				ISC_LIST_DEQUEUE(session->cstreams, cstream,
 						 link);
 				cstream->read_cb(
-					session->handle, ISC_R_UNEXPECTED,
+					session->client_httphandle,
+					ISC_R_UNEXPECTED,
 					&(isc_region_t){ cstream->rbuf,
 							 cstream->rbufsize },
 					cstream->read_cbarg);
@@ -437,6 +441,10 @@ finish_http_session(isc_nm_http_session_t *session) {
 		isc_nmhandle_detach(&session->handle);
 	}
 
+	if (session->client_httphandle != NULL) {
+		isc_nmhandle_detach(&session->client_httphandle);
+	}
+
 	INSIST(ISC_LIST_EMPTY(session->cstreams));
 
 	/* detach from server socket */
@@ -1049,15 +1057,24 @@ get_http_cstream(isc_nmsocket_t *sock, http_cstream_t **streamp) {
 }
 
 static void
-http_call_connect_cb(isc_nmsocket_t *sock, isc_result_t result) {
+http_call_connect_cb(isc_nmsocket_t *sock, isc_nm_http_session_t *session,
+		     isc_result_t result) {
 	isc__nm_uvreq_t *req = NULL;
+	isc_nmhandle_t *httphandle = isc__nmhandle_get(sock, &sock->peer,
+						       &sock->iface->addr);
 
 	REQUIRE(sock->connect_cb != NULL);
 
 	req = isc__nm_uvreq_get(sock->mgr, sock);
 	req->cb.connect = sock->connect_cb;
 	req->cbarg = sock->connect_cbarg;
-	req->handle = isc__nmhandle_get(sock, &sock->peer, &sock->iface->addr);
+	if (session != NULL) {
+		session->client_httphandle = httphandle;
+		req->handle = NULL;
+		isc_nmhandle_attach(httphandle, &req->handle);
+	} else {
+		req->handle = httphandle;
+	}
 
 	isc__nmsocket_clearcb(sock);
 	isc__nm_connectcb(sock, req, result, true);
@@ -1066,8 +1083,8 @@ http_call_connect_cb(isc_nmsocket_t *sock, isc_result_t result) {
 static void
 transport_connect_cb(isc_nmhandle_t *handle, isc_result_t result, void *cbarg) {
 	isc_nmsocket_t *http_sock = (isc_nmsocket_t *)cbarg;
-	isc_nm_http_session_t *session = NULL;
 	isc_nmsocket_t *transp_sock = NULL;
+	isc_nm_http_session_t *session = NULL;
 	http_cstream_t *cstream = NULL;
 	isc_mem_t *mctx = NULL;
 
@@ -1134,13 +1151,15 @@ transport_connect_cb(isc_nmhandle_t *handle, isc_result_t result, void *cbarg) {
 	}
 
 	http_transpost_tcp_nodelay(handle);
-	http_call_connect_cb(http_sock, result);
+
+	http_call_connect_cb(http_sock, session, result);
+
 	http_do_bio(session, NULL, NULL, NULL);
 	isc__nmsocket_detach(&http_sock);
 	return;
 
 error:
-	http_call_connect_cb(http_sock, result);
+	http_call_connect_cb(http_sock, session, result);
 
 	if (http_sock->h2.connect.uri != NULL) {
 		isc_mem_free(mctx, http_sock->h2.connect.uri);
@@ -2361,7 +2380,7 @@ failed_read_cb(isc_result_t result, isc_nm_http_session_t *session) {
 		cstream = ISC_LIST_HEAD(session->cstreams);
 		while (cstream != NULL) {
 			http_cstream_t *next = ISC_LIST_NEXT(cstream, link);
-			cstream->read_cb(session->handle, result,
+			cstream->read_cb(session->client_httphandle, result,
 					 &(isc_region_t){ cstream->rbuf,
 							  cstream->rbufsize },
 					 cstream->read_cbarg);
diff --git a/lib/isc/tests/doh_test.c b/lib/isc/tests/doh_test.c
index d5f4c83e34..ec7c8883c9 100644
--- a/lib/isc/tests/doh_test.c
+++ b/lib/isc/tests/doh_test.c
@@ -581,20 +581,13 @@ timeout_retry_cb(isc_nmhandle_t *handle, isc_result_t eresult,
 		return;
 	}
 
-	/*
-	 * XXX: We should be attaching to a readhandle in
-	 * timeout_request_cb() and detaching it here, but we
-	 * don't do this because the handle passed to this function
-	 * is for the tcpsocket, not the httpsocket. This is a
-	 * bug.
-	 */
-	/* isc_nmhandle_detach(&handle); */
+	isc_nmhandle_detach(&handle);
 }
 
 static void
 timeout_request_cb(isc_nmhandle_t *handle, isc_result_t result, void *arg) {
 	isc_nmhandle_t *sendhandle = NULL;
-	/* isc_nmhandle_t *readhandle = NULL; */
+	isc_nmhandle_t *readhandle = NULL;
 
 	REQUIRE(VALID_NMHANDLE(handle));
 
@@ -608,7 +601,7 @@ timeout_request_cb(isc_nmhandle_t *handle, isc_result_t result, void *arg) {
 				     .length = send_msg.len },
 		    timeout_query_sent_cb, arg);
 
-	/* isc_nmhandle_attach(handle, &readhandle); */
+	isc_nmhandle_attach(handle, &readhandle);
 	isc_nm_read(handle, timeout_retry_cb, NULL);
 	return;