git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jeff King <peff@peff.net>
To: Junio C Hamano <gitster@pobox.com>
Cc: git@vger.kernel.org, Ramsay Jones <ramsay@ramsayjones.plus.com>
Subject: [PATCH 3/3] http: support CURLOPT_PROTOCOLS_STR
Date: Sun, 15 Jan 2023 15:12:13 -0500	[thread overview]
Message-ID: <Y8ReHbGWetJHQcI1@coredump.intra.peff.net> (raw)
In-Reply-To: <Y8RddcM9Vr71ljp4@coredump.intra.peff.net>

The CURLOPT_PROTOCOLS (and matching CURLOPT_REDIR_PROTOCOLS) flag was
deprecated in curl 7.85.0, and using it generate compiler warnings as of
curl 7.87.0. The path forward is to use CURLOPT_PROTOCOLS_STR, but we
can't just do so unilaterally, as it was only introduced less than a
year ago in 7.85.0.

Until that version becomes ubiquitous, we have to either disable the
deprecation warning or conditionally use the "STR" variant on newer
versions of libcurl. This patch switches to the new variant, which is
nice for two reasons:

  - we don't have to worry that silencing curl's deprecation warnings
    might cause us to miss other more useful ones

  - we'd eventually want to move to the new variant anyway, so this gets
    us set up (albeit with some extra ugly boilerplate for the
    conditional)

There are a lot of ways to split up the two cases. One way would be to
abstract the storage type (strbuf versus a long), how to append
(strbuf_addstr vs bitwise OR), how to initialize, which CURLOPT to use,
and so on. But the resulting code looks pretty magical:

  GIT_CURL_PROTOCOL_TYPE allowed = GIT_CURL_PROTOCOL_TYPE_INIT;
  if (...http is allowed...)
	GIT_CURL_PROTOCOL_APPEND(&allowed, "http", CURLOPT_HTTP);

and you end up with more "#define GIT_CURL_PROTOCOL_TYPE" macros than
actual code.

On the other end of the spectrum, we could just implement two separate
functions, one that handles a string list and one that handles bits. But
then we end up repeating our list of protocols (http, https, ftp, ftp).

This patch takes the middle ground. The run-time code is always there to
handle both types, and we just choose which one to feed to curl.

Signed-off-by: Jeff King <peff@peff.net>
---
 git-curl-compat.h |  8 ++++++++
 http.c            | 41 ++++++++++++++++++++++++++++++++++-------
 2 files changed, 42 insertions(+), 7 deletions(-)

diff --git a/git-curl-compat.h b/git-curl-compat.h
index 56a83b6bbd..fd96b3cdff 100644
--- a/git-curl-compat.h
+++ b/git-curl-compat.h
@@ -126,4 +126,12 @@
 #define GIT_CURL_HAVE_CURLSSLSET_NO_BACKENDS
 #endif
 
+/**
+ * CURLOPT_PROTOCOLS_STR and CURLOPT_REDIR_PROTOCOLS_STR were added in 7.85.0,
+ * released in August 2022.
+ */
+#if LIBCURL_VERSION_NUM >= 0x075500
+#define GIT_CURL_HAVE_CURLOPT_PROTOCOLS_STR 1
+#endif
+
 #endif
diff --git a/http.c b/http.c
index ca0fe80ddb..e529ebc993 100644
--- a/http.c
+++ b/http.c
@@ -764,18 +764,29 @@ void setup_curl_trace(CURL *handle)
 	curl_easy_setopt(handle, CURLOPT_DEBUGDATA, NULL);
 }
 
-static long get_curl_allowed_protocols(int from_user)
+static void proto_list_append(struct strbuf *list_str, const char *proto_str,
+			      long *list_bits, long proto_bits)
+{
+	*list_bits |= proto_bits;
+	if (list_str) {
+		if (list_str->len)
+			strbuf_addch(list_str, ',');
+		strbuf_addstr(list_str, proto_str);
+	}
+}
+
+static long get_curl_allowed_protocols(int from_user, struct strbuf *list)
 {
 	long allowed_protocols = 0;
 
 	if (is_transport_allowed("http", from_user))
-		allowed_protocols |= CURLPROTO_HTTP;
+		proto_list_append(list, "http", &allowed_protocols, CURLPROTO_HTTP);
 	if (is_transport_allowed("https", from_user))
-		allowed_protocols |= CURLPROTO_HTTPS;
+		proto_list_append(list, "https", &allowed_protocols, CURLPROTO_HTTPS);
 	if (is_transport_allowed("ftp", from_user))
-		allowed_protocols |= CURLPROTO_FTP;
+		proto_list_append(list, "ftp", &allowed_protocols, CURLPROTO_FTP);
 	if (is_transport_allowed("ftps", from_user))
-		allowed_protocols |= CURLPROTO_FTPS;
+		proto_list_append(list, "ftps", &allowed_protocols, CURLPROTO_FTPS);
 
 	return allowed_protocols;
 }
@@ -921,10 +932,26 @@ static CURL *get_curl_handle(void)
 
 	curl_easy_setopt(result, CURLOPT_MAXREDIRS, 20);
 	curl_easy_setopt(result, CURLOPT_POSTREDIR, CURL_REDIR_POST_ALL);
+
+#ifdef GIT_CURL_HAVE_CURLOPT_PROTOCOLS_STR
+	{
+		struct strbuf buf = STRBUF_INIT;
+
+		get_curl_allowed_protocols(0, &buf);
+		curl_easy_setopt(result, CURLOPT_REDIR_PROTOCOLS_STR, buf.buf);
+		strbuf_reset(&buf);
+
+		get_curl_allowed_protocols(-1, &buf);
+		curl_easy_setopt(result, CURLOPT_PROTOCOLS_STR, buf.buf);
+		strbuf_release(&buf);
+	}
+#else
 	curl_easy_setopt(result, CURLOPT_REDIR_PROTOCOLS,
-			 get_curl_allowed_protocols(0));
+			 get_curl_allowed_protocols(0, NULL));
 	curl_easy_setopt(result, CURLOPT_PROTOCOLS,
-			 get_curl_allowed_protocols(-1));
+			 get_curl_allowed_protocols(-1, NULL));
+#endif
+
 	if (getenv("GIT_CURL_VERBOSE"))
 		http_trace_curl_no_data();
 	setup_curl_trace(result);
-- 
2.39.0.513.g00e40dbe01

  parent reply	other threads:[~2023-01-15 20:12 UTC|newest]

Thread overview: 42+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-01-14  3:47 [PATCH] ci: do not die on deprecated-declarations warning Junio C Hamano
2023-01-14 14:29 ` Ramsay Jones
2023-01-14 15:14   ` Ramsay Jones
2023-01-14 16:13   ` Junio C Hamano
2023-01-14 14:47 ` Jeff King
2023-01-14 14:57   ` Jeff King
2023-01-14 16:15   ` Junio C Hamano
2023-01-14 17:15     ` Jeff King
2023-01-15  6:59       ` Junio C Hamano
2023-01-15 20:08         ` Jeff King
2023-01-15 21:38           ` Junio C Hamano
2023-01-14 16:55   ` Junio C Hamano
2023-01-15  7:02     ` Junio C Hamano
2023-01-15 20:09       ` Jeff King
2023-01-15 20:10         ` [PATCH 1/3] http-push: prefer CURLOPT_UPLOAD to CURLOPT_PUT Jeff King
2023-01-15 20:54           ` Ramsay Jones
2023-01-15 23:13             ` Jeff King
2023-01-15 23:49               ` Jeff King
2023-01-15 20:10         ` [PATCH 2/3] http: prefer CURLOPT_SEEKFUNCTION to CURLOPT_IOCTLFUNCTION Jeff King
2023-01-15 21:11           ` Ramsay Jones
2023-01-15 21:45           ` Junio C Hamano
2023-01-15 23:17             ` Jeff King
2023-01-15 20:12         ` Jeff King [this message]
2023-01-15 21:37           ` [PATCH 3/3] http: support CURLOPT_PROTOCOLS_STR Ramsay Jones
2023-01-15 23:22             ` Jeff King
2023-01-16 13:06           ` Ævar Arnfjörð Bjarmason
2023-01-16 16:05             ` Junio C Hamano
2023-01-16 16:26               ` Junio C Hamano
2023-01-16 17:23               ` Jeff King
2023-01-16 17:27             ` Jeff King
2023-01-17  3:03         ` [PATCH v2] avoiding deprecated curl options Jeff King
2023-01-17  3:04           ` [PATCH v2 1/3] http-push: prefer CURLOPT_UPLOAD to CURLOPT_PUT Jeff King
2023-01-17  3:04           ` [PATCH v2 2/3] http: prefer CURLOPT_SEEKFUNCTION to CURLOPT_IOCTLFUNCTION Jeff King
2023-01-17  3:04           ` [PATCH v2 3/3] http: support CURLOPT_PROTOCOLS_STR Jeff King
2023-01-18  1:03           ` [PATCH v2] avoiding deprecated curl options Ramsay Jones
2023-01-14 14:56 ` [PATCH] ci: do not die on deprecated-declarations warning Jeff King
2023-01-16  0:39   ` Ramsay Jones
2023-01-16 17:13     ` Jeff King
2023-01-14 17:13 ` [PATCH v2] " Junio C Hamano
2023-01-14 17:17   ` Jeff King
2023-01-17  3:03     ` [PATCH 1/3] http-push: prefer CURLOPT_UPLOAD to CURLOPT_PUT Jeff King
2023-01-17  3:04       ` Jeff King

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=Y8ReHbGWetJHQcI1@coredump.intra.peff.net \
    --to=peff@peff.net \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=ramsay@ramsayjones.plus.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).