git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH 1/2] http: Fix handling of missing CURLPROTO_*

From
Junio C Hamano <gitster@pobox.com>
Date
Aug 12, 2017, 00:30 UTC
Message-ID
<xmqqo9rly6dx.fsf@gitster.mtv.corp.google.com>
In-Reply-To
<4d29d43d458f61c6dabca093f591ad8698ca2ceb.1502462884.git.tgc@jupiterrise.com>
"Tom G. Christensen" <tgc@jupiterrise.com> writes:
Show 42 quoted lines
> Commit aeae4db1 refactored the handling of the curl protocol restriction
> support into a function but failed to add a version check for older
> versions of curl that lack CURLPROTO_* support.
> This adds the missing check and at the same time converts it to a feature
> check instead of a version based check.
> This is done to ensure that vendor supported curl versions that have had
> CURLPROTO_* support backported are handled correctly.
>
> Signed-off-by: Tom G. Christensen <tgc@jupiterrise.com>
> ---
>  http.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/http.c b/http.c
> index e00264cff..569909e8a 100644
> --- a/http.c
> +++ b/http.c
> @@ -685,6 +685,7 @@ void setup_curl_trace(CURL *handle)
>  	curl_easy_setopt(handle, CURLOPT_DEBUGDATA, NULL);
>  }
>  
> +#ifdef CURLPROTO_HTTP
>  static long get_curl_allowed_protocols(int from_user)
>  {
>  	long allowed_protocols = 0;
> @@ -700,6 +701,7 @@ static long get_curl_allowed_protocols(int from_user)
>  
>  	return allowed_protocols;
>  }
> +#endif
>  
>  static CURL *get_curl_handle(void)
>  {
> @@ -798,7 +800,7 @@ static CURL *get_curl_handle(void)
>  #elif LIBCURL_VERSION_NUM >= 0x071101
>  	curl_easy_setopt(result, CURLOPT_POST301, 1);
>  #endif
> -#if LIBCURL_VERSION_NUM >= 0x071304
> +#ifdef CURLPROTO_HTTP
>  	curl_easy_setopt(result, CURLOPT_REDIR_PROTOCOLS,
>  			 get_curl_allowed_protocols(0));
>  	curl_easy_setopt(result, CURLOPT_PROTOCOLS,

This may make the code to _compile_, but is it sensible to let the code build and be used by the end users without the "these protocols are safe" filter, I wonder?

Granted, ancient code was unsafe and people were happily using it, but now we know better, and more importantly, we have since added users of transport (e.g. blindly fetch submodules recursively) that may _rely_ on this layer of the code safely filtering unsafe protocols, so...

Previous: Tom G. ChristensenNext: Tom G. Christensen
Message 4 of 11 in “http: handle curl with vendor backports”
  1. 0/2 http: handle curl with vendor backportsTom G. Christensen, Aug 11, 2017
  2. 2/2 http: use a feature check to enable GSSAPI delegation controlTom G. Christensen, Aug 11, 2017
  3. 1/2 http: Fix handling of missing CURLPROTO_*Tom G. Christensen, Aug 11, 2017
  4. Junio C HamanoAug 12, 2017
  5. Tom G. ChristensenAug 12, 2017
  6. Jeff KingAug 20, 2017
  7. Junio C HamanoAug 11, 2017
  8. Tom G. ChristensenAug 12, 2017
  9. Jeff KingAug 20, 2017
  10. Junio C HamanoAug 20, 2017
  11. Jeff KingAug 23, 2017

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.