Re: [PATCH v7 4/4] transport: add from_user parameter to is_transport_allowed
From: Jeff King <hidden>
Date: 2016-12-01 21:40:12
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').
quoted hunk ↗ jump to hunk
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