From: Junio C Hamano <gitster@pobox•com>
To: Knut Franke <k.franke@science-computing•de>
Cc: Eric Sunshine <sunshine@sunshineco•com>, git@vger•kernel.org
Subject: Re: [PATCH 1/2] http: allow selection of proxy authentication method
Date: Fri, 30 Oct 2015 12:19:52 -0700 [thread overview]
Message-ID: <xmqqd1vwkvtz.fsf@gitster.mtv.corp.google.com> (raw)
In-Reply-To: <20151030180129.GA6425@science-computing.de> (Knut Franke's message of "Fri, 30 Oct 2015 19:01:30 +0100")
Knut Franke <k.franke@science-computing•de> writes:
> Maybe add a #define LIBCURL_CAN_HANDLE_PROXY_AUTH to clarify this, like we do
> with LIBCURL_CAN_HANDLE_AUTH_ANY?
Quite frankly, my first preference is to have a code that is clear
enough so that you do not need such an intermediate macro. By
isolating implementation details that have to be version dependent
into a helper function whose purpose is well defined, the rest of
the code can be #ifdef free; it would become sufficiently clear to
switch based on curl version where implementation details matter.
By refraining from littering #ifdef all over the code, we do assign
to disable_gssnegotiate even though the value is not even used when
compiled for ancient version of libCURL, but the benefit of code
clarity outweighs such downside. We may have to use #ifdef/#endif
in some places, but we should in general minimize their uses and
write code for the more up-to-date API.
For what I mean, see the attached patch outline to show how to get
rid of CAN_HANDLE_AUTH_ANY.
> How about env_override? Not perfect, but probably better.
Much better than anything I'd come up with myself (I am bad at
naming, even though I may sometimes be good at spotting a bad name).
http.c | 32 ++++++++++++++++----------------
1 file changed, 16 insertions(+), 16 deletions(-)
diff --git a/http.c b/http.c
index 7da76ed..d272b02 100644
--- a/http.c
+++ b/http.c
@@ -15,10 +15,6 @@ int active_requests;
int http_is_verbose;
size_t http_post_buffer = 16 * LARGE_PACKET_MAX;
-#if LIBCURL_VERSION_NUM >= 0x070a06
-#define LIBCURL_CAN_HANDLE_AUTH_ANY
-#endif
-
static int min_curl_sessions = 1;
static int curl_session_count;
#ifdef USE_CURL_MULTI
@@ -79,9 +75,6 @@ static const char *user_agent;
static struct credential cert_auth = CREDENTIAL_INIT;
static int ssl_cert_password_required;
-#ifdef LIBCURL_CAN_HANDLE_AUTH_ANY
-static unsigned long http_auth_methods = CURLAUTH_ANY;
-#endif
static struct curl_slist *pragma_header;
static struct curl_slist *no_pragma_header;
@@ -90,6 +83,19 @@ static struct active_request_slot *active_queue_head;
static char *cached_accept_language;
+static int disable_gssnegotiate;
+
+static void set_httpauth_opt(CURL *curl)
+{
+#if LIBCURL_VERSION_NUM >= 0x070a06 /* Is CURLAUTH_ANY available? */
+ unsigned long auth_methods = CURLAUTH_ANY;
+
+ if (disable_gssnegotiate)
+ auth_methods &= ~CURLAUTH_GSSNEGOTIATE;
+ curl_easy_setopt(curl, CURLOPT_HTTPAUTH, auth_methods);
+#endif
+}
+
size_t fread_buffer(char *ptr, size_t eltsize, size_t nmemb, void *buffer_)
{
size_t size = eltsize * nmemb;
@@ -375,9 +381,7 @@ static CURL *get_curl_handle(void)
#if LIBCURL_VERSION_NUM >= 0x070907
curl_easy_setopt(result, CURLOPT_NETRC, CURL_NETRC_OPTIONAL);
#endif
-#ifdef LIBCURL_CAN_HANDLE_AUTH_ANY
- curl_easy_setopt(result, CURLOPT_HTTPAUTH, CURLAUTH_ANY);
-#endif
+ set_httpauth_opt(result);
if (http_proactive_auth)
init_curl_http_auth(result);
@@ -681,9 +685,7 @@ struct active_request_slot *get_active_slot(void)
curl_easy_setopt(slot->curl, CURLOPT_UPLOAD, 0);
curl_easy_setopt(slot->curl, CURLOPT_HTTPGET, 1);
curl_easy_setopt(slot->curl, CURLOPT_FAILONERROR, 1);
-#ifdef LIBCURL_CAN_HANDLE_AUTH_ANY
- curl_easy_setopt(slot->curl, CURLOPT_HTTPAUTH, http_auth_methods);
-#endif
+ set_httpauth_opt(slot->curl);
if (http_auth.password)
init_curl_http_auth(slot->curl);
@@ -943,9 +945,7 @@ static int handle_curl_result(struct slot_results *results)
credential_reject(&http_auth);
return HTTP_NOAUTH;
} else {
-#ifdef LIBCURL_CAN_HANDLE_AUTH_ANY
- http_auth_methods &= ~CURLAUTH_GSSNEGOTIATE;
-#endif
+ disable_gssnegotiate = 1;
return HTTP_REAUTH;
}
} else {
next prev parent reply other threads:[~2015-10-30 19:20 UTC|newest]
Thread overview: 34+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-10-26 17:55 [PATCH 1/2] http: allow selection of proxy authentication method Knut Franke
2015-10-26 17:55 ` [PATCH 2/2] http: use credential API to handle proxy authentication Knut Franke
2015-10-26 20:33 ` [PATCH 1/2] http: allow selection of proxy authentication method Junio C Hamano
2015-10-27 8:47 ` Knut Franke
2015-10-28 9:40 ` [PATCH v2] http proxy authentication improvements Knut Franke
2015-10-28 9:40 ` [PATCH 1/2] http: allow selection of proxy authentication method Knut Franke
2015-10-28 16:51 ` Junio C Hamano
2015-10-28 16:59 ` Junio C Hamano
2015-10-30 18:01 ` Knut Franke
2015-10-30 19:19 ` Junio C Hamano [this message]
2015-10-28 18:54 ` Eric Sunshine
2015-10-28 9:40 ` [PATCH 2/2] http: use credential API to handle proxy authentication Knut Franke
2015-10-28 18:58 ` Eric Sunshine
2015-10-30 18:24 ` Knut Franke
2015-10-30 19:31 ` Junio C Hamano
2015-10-30 19:35 ` Eric Sunshine
2015-11-02 16:54 ` [PATCH v3 0/2] Knut Franke
2015-11-02 16:54 ` [PATCH 1/2] http: allow selection of proxy authentication method Knut Franke
2015-11-02 22:46 ` Junio C Hamano
2015-11-03 9:07 ` Knut Franke
2015-11-03 19:46 ` Junio C Hamano
2015-11-02 16:54 ` [PATCH 2/2] http: use credential API to handle proxy authentication Knut Franke
2015-11-02 22:54 ` Junio C Hamano
2015-11-03 9:31 ` Knut Franke
2015-11-03 18:12 ` Eric Sunshine
2015-11-04 9:13 ` [PATCH v4 0/2] Knut Franke
2015-11-04 9:13 ` [PATCH 1/2] http: allow selection of proxy authentication method Knut Franke
2015-11-04 19:42 ` Junio C Hamano
2015-11-04 9:13 ` [PATCH 2/2] http: use credential API to handle proxy authentication Knut Franke
2015-11-04 19:41 ` Eric Sunshine
2015-11-04 19:53 ` Junio C Hamano
2015-11-05 8:24 ` Jeff King
2015-11-05 11:56 ` Knut Franke
2015-11-05 17:30 ` 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=xmqqd1vwkvtz.fsf@gitster.mtv.corp.google.com \
--to=gitster@pobox$(echo .)com \
--cc=git@vger$(echo .)kernel.org \
--cc=k.franke@science-computing$(echo .)de \
--cc=sunshine@sunshineco$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
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