From: Arran Cudbard-Bell <a.cudbardb@freeradius.org>
Date: Thu, 16 May 2024 22:00:28 +0000 (-0600)
Subject: Some minor tweaks to expose problems in rlm_rest
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=199b6f9b51e2772446ccd5067e3ece305670798f;p=thirdparty%2Ffreeradius-server.git

Some minor tweaks to expose problems in rlm_rest
---

diff --git a/src/lib/curl/io.c b/src/lib/curl/io.c
index 64e2a30adc..b72064bb98 100644
--- a/src/lib/curl/io.c
+++ b/src/lib/curl/io.c
@@ -58,7 +58,7 @@ static inline void _fr_curl_io_demux(fr_curl_handle_t *mhandle, CURLM *mandle)
 		case CURLMSG_DONE:
 		{
 			fr_curl_io_request_t	*randle;
-			request_t			*request = NULL;
+			request_t		*request = NULL;
 			CURL			*candle = m->easy_handle;
 			CURLcode		ret;
 
@@ -67,6 +67,17 @@ static inline void _fr_curl_io_demux(fr_curl_handle_t *mhandle, CURLM *mandle)
 			mhandle->transfers--;
 
 			ret = curl_easy_getinfo(candle, CURLINFO_PRIVATE, &randle);
+			/*
+			 *	In curl 7.61 we got bad request data returned
+			 *	here after cancellations were processed.
+			 *
+			 *	For debug builds if the value is equal to
+			 *	0xdeadc341, we know the request was cancelled.
+			 *
+			 *	There is no good work around for this other than
+			 *	upgrading to a newer version of curl.
+			 */
+			talloc_get_type_abort(randle, fr_curl_io_request_t);
 			if (!fr_cond_assert_msg(ret == CURLE_OK,
 						"Failed retrieving request data from CURL easy handle (candle)")) {
 				curl_multi_remove_handle(mandle, candle);
@@ -86,8 +97,8 @@ static inline void _fr_curl_io_demux(fr_curl_handle_t *mhandle, CURLM *mandle)
 			randle->result = m->data.result;
 
 			/*
-			 *	Looks like this needs to be done last,
-			 *	else m->data.result ends up being junk.
+			 *	This needs to be done last, else m->data.result
+			 *	ends up being junk.
 			 */
 			curl_multi_remove_handle(mandle, candle);
 
@@ -491,6 +502,11 @@ int fr_curl_io_request_enqueue(fr_curl_handle_t *mhandle, request_t *request, fr
 	 *	Stick the current request in the curl handle's
 	 *	private data.  This makes it simple to resume
 	 *	the request in the demux function later...
+	 *
+	 *	Note: If you get 0xdeadc341 in the private data
+	 *	in the demux function, the result for the easy
+	 *	handle was erroneously delivered after a
+	 *	cancellation.
 	 */
 	ret = curl_easy_setopt(randle->candle, CURLOPT_PRIVATE, randle);
 	if (ret != CURLE_OK) {
diff --git a/src/modules/rlm_rest/rlm_rest.c b/src/modules/rlm_rest/rlm_rest.c
index afe3a55443..179507b851 100644
--- a/src/modules/rlm_rest/rlm_rest.c
+++ b/src/modules/rlm_rest/rlm_rest.c
@@ -1177,12 +1177,6 @@ static int parse_sub_section(rlm_rest_t *inst, CONF_SECTION *parent, conf_parser
 	return 0;
 }
 
-static int _mod_conn_free(fr_curl_io_request_t *randle)
-{
-	curl_easy_cleanup(randle->candle);
-	return 0;
-}
-
 /** Cleans up after a REST request.
  *
  * Resets all options associated with a CURL handle, and frees any headers
@@ -1194,7 +1188,7 @@ static int _mod_conn_free(fr_curl_io_request_t *randle)
  * @param[in] randle to cleanup.
  * @param[in] uctx unused.
  */
-static int rest_request_cleanup(fr_curl_io_request_t *randle, UNUSED void *uctx)
+static int _rest_request_cleanup(fr_curl_io_request_t *randle, UNUSED void *uctx)
 {
 	rlm_rest_curl_context_t *ctx = talloc_get_type_abort(randle->uctx, rlm_rest_curl_context_t);
 	CURL			*candle = randle->candle;
@@ -1212,6 +1206,15 @@ static int rest_request_cleanup(fr_curl_io_request_t *randle, UNUSED void *uctx)
 		ctx->headers = NULL;
 	}
 
+#ifndef NDEBUG
+	/*
+	 *  With curl 7.61 when a request in cancelled we get a result
+	 *  with a NULL (invalid) pointer to private data.  This lets
+	 *  us know that it was request returned to the slab.
+	 */
+	curl_easy_setopt(candle, CURLOPT_PRIVATE, (void *)0xdeadc341);
+#endif
+
 	/*
 	 *  Free response data
 	 */
@@ -1225,6 +1228,12 @@ static int rest_request_cleanup(fr_curl_io_request_t *randle, UNUSED void *uctx)
 	return 0;
 }
 
+static int _mod_conn_free(fr_curl_io_request_t *randle)
+{
+	curl_easy_cleanup(randle->candle);
+	return 0;
+}
+
 static int rest_conn_alloc(fr_curl_io_request_t *randle, void *uctx)
 {
 	rlm_rest_t const	*inst = talloc_get_type_abort(uctx, rlm_rest_t);
@@ -1244,7 +1253,7 @@ static int rest_conn_alloc(fr_curl_io_request_t *randle, void *uctx)
 	randle->uctx = curl_ctx;
 	talloc_set_destructor(randle, _mod_conn_free);
 
-	rest_slab_element_set_destructor(randle, rest_request_cleanup, NULL);
+	rest_slab_element_set_destructor(randle,  _rest_request_cleanup, NULL);
 
 	return 0;
 }