Re: [PATCH] http.c: fix parsing of http.sslCertPasswordProtected variable

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxx>

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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]