On Thu, Dec 01, 2016 at 12:25:59PM -0800, Brandon Williams wrote:
> Add the from_user parameter to the 'is_transport_allowed' function.
> This allows callers to query if a transport protocol is allowed, given
> that the caller knows that the protocol is coming from the user (1) or
> not from the user (0), such as redirects in libcurl. If unknown, a -1
> should be provided which falls back to reading `GIT_PROTOCOL_FROM_USER`
> to determine if the protocol came from the user.
Patches 3 and 4 look good to me (1 and 2 are unchanged, right? They are
already in 'next' anyway, though I guess we are due for a post-release
reset of 'next').
> diff --git a/http.c b/http.c
> index fee128b..e74c0f0 100644
> --- a/http.c
> +++ b/http.c
> @@ -725,13 +725,13 @@ static CURL *get_curl_handle(void)
> curl_easy_setopt(result, CURLOPT_POST301, 1);
> #endif
> #if LIBCURL_VERSION_NUM >= 0x071304
> - if (is_transport_allowed("http"))
> + if (is_transport_allowed("http", 0))
> allowed_protocols |= CURLPROTO_HTTP;
> - if (is_transport_allowed("https"))
> + if (is_transport_allowed("https", 0))
> allowed_protocols |= CURLPROTO_HTTPS;
> - if (is_transport_allowed("ftp"))
> + if (is_transport_allowed("ftp", 0))
> allowed_protocols |= CURLPROTO_FTP;
> - if (is_transport_allowed("ftps"))
> + if (is_transport_allowed("ftps", 0))
> allowed_protocols |= CURLPROTO_FTPS;
> curl_easy_setopt(result, CURLOPT_REDIR_PROTOCOLS, allowed_protocols);
> #else
This is better, but I think we still need to deal with http-alternates
on top.
I think we'd need to move this allowed_protocols setup into a function
like:
int generate_allowed_protocols(int from_user)
{
int ret;
if (is_transport_allowed("http", from_user))
ret |= CURLPROTO_HTTP;
... etc ...
return ret;
}
and then create a protocol list for each situation:
allowed_protocols = generate_allowed_protocols(-1);
allowed_redir_protocols = generate_allowed_protocols(0);
and then we know we can always set up the redir protocols:
curl_easy_setopt(result, CURLOPT_REDIR_PROTOCOLS, allowed_redir_protocols);
and which we feed for CURLOPT_PROTOCOLS depends on whether we are
following an http-alternates redirect or not. But I suspect it will be a
nasty change to plumb through the idea of "this request is on behalf of
an http-alternates redirect".
Given how few people probably care, I'm tempted to document it as a
quirk and direct people to the upcoming http.followRedirects. The newly
proposed default value of that disables http-alternates entirely anyway.
-Peff