[v6,1/1] http: add support selecting http version

Commit Message

Linus Arver via GitGitGadget Nov. 8, 2018, 7 a.m. UTC

From: Force Charlie <[e-mail]>

Usually we don't need to set libcurl to choose which version of the
HTTP protocol to use to communicate with a server.
But different versions of libcurl, the default value is not the same.

CURL >= 7.62.0: CURL_HTTP_VERSION_2TLS
CURL < 7.62: CURL_HTTP_VERSION_1_1

In order to give users the freedom to control the HTTP version,
we need to add a setting to choose which HTTP version to use.

Signed-off-by: Force Charlie <[e-mail]>
---
 Documentation/config.txt |  9 +++++++++
 http.c                   | 38 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 47 insertions(+)

Comments

Eric Sunshine Nov. 8, 2018, 6:02 p.m. UTC | #1

On Thu, Nov 8, 2018 at 2:00 AM Force Charlie via GitGitGadget
<[e-mail]> wrote:
> In order to give users the freedom to control the HTTP version,
> we need to add a setting to choose which HTTP version to use.
>
> Signed-off-by: Force Charlie <[e-mail]>
> ---
> diff --git a/http.c b/http.c
> @@ -284,6 +285,9 @@ static void process_curl_messages(void)
>  static int http_options(const char *var, const char *value, void *cb)
>  {
> +       if (!strcmp("http.version",var)) {

Style: space after comma

> +               return git_config_string(&curl_http_version, var, value);
> +       }
> @@ -806,6 +834,16 @@ static CURL *get_curl_handle(void)
> +    if (curl_http_version) {
> +               long opt;
> +               if (!get_curl_http_version_opt(curl_http_version, &opt)) {
> +                       /* Set request use http version */
> +                       curl_easy_setopt(result, CURLOPT_HTTP_VERSION,opt);

Style: space after comma

> +               }
> +    }

Junio C Hamano Nov. 9, 2018, 2:56 a.m. UTC | #2

"Force Charlie via GitGitGadget" <[e-mail]> writes:

> +#if LIBCURL_VERSION_NUM >=0x072f00
> +static int get_curl_http_version_opt(const char *version_string, long *opt)
> +{
> +	int i;
> +	static struct {
> +		const char *name;
> +		long opt_token;
> +	} choice[] = {
> +		{ "HTTP/1.1", CURL_HTTP_VERSION_1_1 },
> +		{ "HTTP/2", CURL_HTTP_VERSION_2 }
> +	};
> +
> +	for (i = 0; i < ARRAY_SIZE(choice); i++) {
> +		if (!strcmp(version_string, choice[i].name)) {
> +			*opt = choice[i].opt_token;
> +			return 0;
> +		}
> +	}
> +

I wonder if we need to give a warning here about an unknown and
ignored value, by calling something like

	warning("unknown value given to http.version: '%s'", version_string);

here.  We should not trigger noisy warning while reading the
configuration file looking for other variables unrelated to
http.version but this codepath is followed only when we know
we need to find out what value the variable is set to, so it
probably is a good thing to do.  

Thoughts?

Junio C Hamano Nov. 9, 2018, 2:57 a.m. UTC | #3

Eric Sunshine <[e-mail]> writes:

>> @@ -284,6 +285,9 @@ static void process_curl_messages(void)
>>  static int http_options(const char *var, const char *value, void *cb)
>>  {
>> +       if (!strcmp("http.version",var)) {
>
> Style: space after comma
>
>> +               return git_config_string(&curl_http_version, var, value);
>> +       }
>> @@ -806,6 +834,16 @@ static CURL *get_curl_handle(void)
>> +    if (curl_http_version) {
>> +               long opt;
>> +               if (!get_curl_http_version_opt(curl_http_version, &opt)) {
>> +                       /* Set request use http version */
>> +                       curl_easy_setopt(result, CURLOPT_HTTP_VERSION,opt);
>
> Style: space after comma
>
>> +               }
>> +    }

Thanks, both.  This is almost done, I think.

Patch

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 41a9ff2b6a..f397942128 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1935,6 +1935,15 @@  http.saveCookies::
 	If set, store cookies received during requests to the file specified by
 	http.cookieFile. Has no effect if http.cookieFile is unset.
 
+http.version::
+	Use the specified HTTP protocol version when communicating with a server.
+	If you want to force the default. The available and default version depend
+	on libcurl. Actually the possible values of
+	this option are:
+
+	- HTTP/2
+	- HTTP/1.1
+
 http.sslVersion::
 	The SSL version to use when negotiating an SSL connection, if you
 	want to force the default.  The available and default version
diff --git a/http.c b/http.c
index 3dc8c560d6..d6f3c4ee80 100644
--- a/http.c
+++ b/http.c
@@ -48,6 +48,7 @@  char curl_errorstr[CURL_ERROR_SIZE];
 
 static int curl_ssl_verify = -1;
 static int curl_ssl_try;
+static const char *curl_http_version = NULL;
 static const char *ssl_cert;
 static const char *ssl_cipherlist;
 static const char *ssl_version;
@@ -284,6 +285,9 @@  static void process_curl_messages(void)
 
 static int http_options(const char *var, const char *value, void *cb)
 {
+	if (!strcmp("http.version",var)) {
+		return git_config_string(&curl_http_version, var, value);
+	}
 	if (!strcmp("http.sslverify", var)) {
 		curl_ssl_verify = git_config_bool(var, value);
 		return 0;
@@ -789,6 +793,30 @@  static long get_curl_allowed_protocols(int from_user)
 }
 #endif
 
+#if LIBCURL_VERSION_NUM >=0x072f00
+static int get_curl_http_version_opt(const char *version_string, long *opt)
+{
+	int i;
+	static struct {
+		const char *name;
+		long opt_token;
+	} choice[] = {
+		{ "HTTP/1.1", CURL_HTTP_VERSION_1_1 },
+		{ "HTTP/2", CURL_HTTP_VERSION_2 }
+	};
+
+	for (i = 0; i < ARRAY_SIZE(choice); i++) {
+		if (!strcmp(version_string, choice[i].name)) {
+			*opt = choice[i].opt_token;
+			return 0;
+		}
+	}
+
+	return -1; /* not found */
+}
+
+#endif
+
 static CURL *get_curl_handle(void)
 {
 	CURL *result = curl_easy_init();
@@ -806,6 +834,16 @@  static CURL *get_curl_handle(void)
 		curl_easy_setopt(result, CURLOPT_SSL_VERIFYHOST, 2);
 	}
 
+#if LIBCURL_VERSION_NUM >= 0x072f00 // 7.47.0
+    if (curl_http_version) {
+		long opt;
+		if (!get_curl_http_version_opt(curl_http_version, &opt)) {
+			/* Set request use http version */
+			curl_easy_setopt(result, CURLOPT_HTTP_VERSION,opt);
+		}
+    }
+#endif
+
 #if LIBCURL_VERSION_NUM >= 0x070907
 	curl_easy_setopt(result, CURLOPT_NETRC, CURL_NETRC_OPTIONAL);
 #endif