Re: [PATCH v2 1/2] remote-curl: accept all encodings supported by curl

From: Junio C Hamano <gitster@pobox.com>
Date: May 23, 2018, 01:23 UTC
Message-ID: <xmqqwovvw4fl.fsf@gitster-ct.c.googlers.com>
In-Reply-To: <20180522184204.47332-1-bmwill@google.com>

Brandon Williams <bmwill@google.com> writes:

Show 11 quoted lines

> diff --git a/t/t5551-http-fetch-smart.sh b/t/t5551-http-fetch-smart.sh
> index f5721b4a5..913089b14 100755
> --- a/t/t5551-http-fetch-smart.sh
> +++ b/t/t5551-http-fetch-smart.sh
> @@ -26,14 +26,14 @@ setup_askpass_helper
>  cat >exp <<EOF
>  > GET /smart/repo.git/info/refs?service=git-upload-pack HTTP/1.1
>  > Accept: */*
> -> Accept-Encoding: gzip
> +> Accept-Encoding: ENCODINGS
>  > Pragma: no-cache

Is the ordering of these headers determined by the user of cURL library (i.e. Git), or whatever the version of cURL we happened to link with happens to produce?

The point is whether the order is expected to be stable, or we are better off sorting the actual log before comparing.

>  < HTTP/1.1 200 OK
>  < Pragma: no-cache
>  < Cache-Control: no-cache, max-age=0, must-revalidate
>  < Content-Type: application/x-git-upload-pack-advertisement

A similar question for this response.

Show 6 quoted lines

>  > POST /smart/repo.git/git-upload-pack HTTP/1.1
> -> Accept-Encoding: gzip
> +> Accept-Encoding: ENCODINGS
>  > Content-Type: application/x-git-upload-pack-request
>  > Accept: application/x-git-upload-pack-result
>  > Content-Length: xxx

Ditto for this request.

Show 14 quoted lines

> @@ -79,8 +79,13 @@ test_expect_success 'clone http repository' '
>  		/^< Date: /d
>  		/^< Content-Length: /d
>  		/^< Transfer-Encoding: /d
> -	" >act &&
> -	test_cmp exp act
> +	" >actual &&
> +	sed -e "s/^> Accept-Encoding: .*/> Accept-Encoding: ENCODINGS/" \
> +			actual >actual.smudged &&
> +	test_cmp exp actual.smudged &&
> +
> +	grep "Accept-Encoding:.*gzip" actual >actual.gzip &&
> +	test_line_count = 2 actual.gzip
>  '

Similarly, how much control do we have to ensure that the test HTTPD server (1) supports gzip and (2) does not support encoding algos with confusing names e.g. "funnygzipalgo" that may accidentally match that pattern?

Thanks. Not a new issue with this patch, but just being curious if you or anybody thought about it as a possible issue.