From: Jonathan Nieder <jrnieder@gmail.com>
Date: Fri, 12 Jul 2013 19:05:31 GMT
Subject: Re: [PATCH] http.c: fix parsing of http.sslCertPasswordProtected variable
Message-ID: <20130712190531.GD8482@google.com>
In-Reply-To: <7v38rjlif4.fsf@alter.siamese.dyndns.org>

Junio C Hamano wrote:

> The existing code triggers only when the configuration variable is
> set to true.  Once the variable is set to true in a more generic
> configuration file (e.g. ~/.gitconfig), it cannot be overriden to
> false in the repository specific one (e.g. .git/config).
[...]
> --- a/http.c
> +++ b/http.c
> @@ -160,8 +160,7 @@ static int http_options(const char *var, const char *value, void *cb)
>  	if (!strcmp("http.sslcainfo", var))
>  		return git_config_string(&ssl_cainfo, var, value);
>  	if (!strcmp("http.sslcertpasswordprotected", var)) {
> -		if (git_config_bool(var, value))
> -			ssl_cert_password_required = 1;
> +		ssl_cert_password_required = git_config_bool(var, value);

Thanks for catching it.  The documentation doesn't say anything about
this "can only enable and cannot disable" behavior and the usual
pattern is to allow later settings to override earlier ones, so this
change looks good.

Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>

FWIW the GIT_SSL_CERT_PASSWORD_PROTECTED envvar has a similar "can
only enable" behavior, but since it's documented, that's not as big
of a problem.  Do you remember why it was written that way?

When that setting was first added[1], there was some mention of
autodetection if libcurl could learn to do that.  Did it happen?
(Please forgive my ignorance.)

Thanks,
Jonathan

[1] http://thread.gmane.org/gmane.comp.version-control.git/122793/focus=122816