In addition to Junio's review comments...

On Wednesday, October 28, 2015, Knut Franke
<k.fra...@science-computing.de> wrote:
> CURLAUTH_ANY does not work with proxies which answer unauthenticated requests
> with a 307 redirect to an error page instead of a 407 listing supported
> authentication methods. Therefore, allow the authentication method to be set
> using the environment variable GIT_HTTP_PROXY_AUTHMETHOD or configuration
> variables http.proxyAuthmethod and remote.<name>.proxyAuthmethod (in analogy
> to http.proxy and remote.<name>.proxy).
>
> The following values are supported:
>
> * anyauth (default)
> * basic
> * digest
> * negotiate
> * ntlm
>
> Signed-off-by: Knut Franke <k.fra...@science-computing.de>
> ---
> diff --git a/Documentation/config.txt b/Documentation/config.txt
> index 391a0c3..f2644d1 100644
> --- a/Documentation/config.txt
> +++ b/Documentation/config.txt
> @@ -1597,6 +1597,29 @@ http.proxy::
>         `curl(1)`).  This can be overridden on a per-remote basis; see
>         remote.<name>.proxy
>
> +http.proxyAuthmethod::

Should this be typed as "proxyAuthMethod"?

> +       Set the method with which to authenticate against the HTTP proxy. 
> This only
> +    takes effect if the configured proxy URI contains a user name part (i.e. 
> is
> +    of the form 'user@host' or 'user@host:port'). This can be overridden on a
> +    per-remote basis; see `remote.<name>.proxyAuthmethod`. Both can be
> +    overridden by the 'GIT_HTTP_PROXY_AUTHMETHOD' environment variable.
> +    Possible values are:
> ++
> +--
> +* `anyauth` - Automatically pick a suitable authentication method. It is
> +  assumed that the proxy answers an unauthenticated request with a 407
> +  status code and one or more Proxy-authenticate headers with supported
> +  authentication methods. This is the default.
> +* `basic` - HTTP Basic authentication
> +* `digest` - HTTP Digest authentication; this prevents the password from 
> being
> +  transmitted to the proxy in clear text
> +* `negotiate` - GSS-Negotiate authentication (compare the --negotiate option
> +  of `curl(1)`)
> +* `ntlm` - NTLM authentication (compare the --ntlm option of `curl(1)`)
> +--

I think you can drop the unnecessary '--' lines here and above.

> ++
> +

No need for the extra unnecessary "+" line and empty line.

> +
>  http.cookieFile::
>         File containing previously stored cookie lines which should be used
>         in the Git http session, if they match the server. The file format
> diff --git a/http.c b/http.c
> index 7da76ed..4756bab 100644
> --- a/http.c
> +++ b/http.c
> @@ -305,6 +324,37 @@ static void init_curl_http_auth(CURL *result)
> +static void init_curl_proxy_auth(CURL *result)
> +{
> +       copy_from_env(&http_proxy_authmethod, "GIT_HTTP_PROXY_AUTHMETHOD");
> +
> +       if (http_proxy_authmethod) {
> +               int i;
> +               for (i = 0; i < ARRAY_SIZE(http_proxy_authmethods); i++) {
> +                       if (!strcmp(http_proxy_authmethod, 
> http_proxy_authmethods[i].name)) {
> +                               curl_easy_setopt(result, CURLOPT_PROXYAUTH,
> +                                               
> http_proxy_authmethods[i].curlauth_param);
> +                               break;
> +                       }
> +               }
> +               if (i == ARRAY_SIZE(http_proxy_authmethods)) {
> +                       warning("unsupported proxy authentication method %s: 
> using default",
> +                             http_proxy_authmethod);

Does the user know what "default" means here? Does it mean
CURLAUTH_ANY? If so, do you need to invoke curl_easy_setopt(...,
CURLAUTH_ANY) as you do below when http_proxy is NULL?

> +               }
> +       }
> +#ifdef LIBCURL_CAN_HANDLE_AUTH_ANY
> +       else
> +               curl_easy_setopt(result, CURLOPT_PROXYAUTH, CURLAUTH_ANY);
> +#endif
> +}
> +
>  static int has_cert_password(void)
>  {
>         if (ssl_cert == NULL || ssl_cert_password_required != 1)
> @@ -466,9 +516,7 @@ static CURL *get_curl_handle(void)
>         if (curl_http_proxy) {
>                 curl_easy_setopt(result, CURLOPT_PROXY, curl_http_proxy);
>         }
> -#if LIBCURL_VERSION_NUM >= 0x070a07
> -       curl_easy_setopt(result, CURLOPT_PROXYAUTH, CURLAUTH_ANY);
> -#endif
> +       init_curl_proxy_auth(result);
>
>         set_curl_keepalive(result);
>
> diff --git a/remote.c b/remote.c
> index 1101f82..426c6d8 100644
> --- a/remote.c
> +++ b/remote.c
> @@ -427,6 +427,9 @@ static int handle_config(const char *key, const char 
> *value, void *cb)
>         } else if (!strcmp(subkey, ".proxy")) {
>                 return git_config_string((const char **)&remote->http_proxy,
>                                          key, value);
> +       } else if (!strcmp(subkey, ".proxyAuthmethod")) {

In documentation, write "proxyAuthMethod", but in code use
"proxyauthmethod" string literal.

> +               return git_config_string((const char 
> **)&remote->http_proxy_authmethod,
> +                                        key, value);
>         } else if (!strcmp(subkey, ".vcs")) {
>                 return git_config_string(&remote->foreign_vcs, key, value);
>         }
> diff --git a/remote.h b/remote.h
> index 312b7ca..a221c5a 100644
> --- a/remote.h
> +++ b/remote.h
> @@ -54,6 +54,7 @@ struct remote {
>          * for curl remotes only
>          */
>         char *http_proxy;
> +       char *http_proxy_authmethod;
>  };
>
>  struct remote *remote_get(const char *name);
> --
> 2.3.7
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to