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

Re: [PATCH 1/2] t5516/t5601: avoid using `localhost` for failing HTTPS requests

From
Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date
Oct 31, 2022, 20:49 UTC
Message-ID
<221031.86v8nz67jn.gmgdl@evledraar.gmail.com>
In-Reply-To
<25cc0f6d91a9d23eb1b755e1463d672e4958a4e9.1667245639.git.gitgitgadget@gmail.com>
On Mon, Oct 31 2022, Johannes Schindelin via GitGitGadget wrote:
Show 55 quoted lines
> From: Johannes Schindelin <johannes.schindelin@gmx.de>
>
> In 6dcbdc0d6616 (remote: create fetch.credentialsInUrl config,
> 2022-06-06), we added four test cases that validate various behavior
> around passing credentials as part of the URL (which is considered
> unsafe in general).
>
> These tests do not _actually_ try to connect anywhere, but have to use
> the https:// protocol in order to validate the intended code paths.
>
> However, using `localhost` for such a connection causes several
> problems:
>
> - There might be a web server running on localhost, and we do not
>   actually want to connect to that.
>
> - The DNS resolver, or the local firewall, might take a substantial
>   amount of time (or forever, whichever comes first) to fail to connect,
>   slowing down the test cases unnecessarily.
>
> Let's instead use an IPv4 address that is guaranteed never to offer a
> web server: 224.0.0.1 (which is part of the IP multicast range).
>
> Incidentally, this seems to fix an issue where the tests fail in the
> Windows jobs of Git's CI builds.
> [...]
> diff --git a/t/t5601-clone.sh b/t/t5601-clone.sh
> index 45f0803ed4d..0b386c74818 100755
> --- a/t/t5601-clone.sh
> +++ b/t/t5601-clone.sh
> @@ -72,19 +72,19 @@ test_expect_success 'clone respects GIT_WORK_TREE' '
>  '
>  
>  test_expect_success LIBCURL 'clone warns or fails when using username:password' '
> -	message="URL '\''https://username:<redacted>@localhost/'\'' uses plaintext credentials" &&
> -	test_must_fail git -c transfer.credentialsInUrl=allow clone https://username:password@localhost attempt1 2>err &&
> +	message="URL '\''https://username:<redacted>@224.0.0.1/'\'' uses plaintext credentials" &&
> +	test_must_fail git -c transfer.credentialsInUrl=allow clone https://username:password@224.0.0.1 attempt1 2>err &&
>  	! grep "$message" err &&
>  
> -	test_must_fail git -c transfer.credentialsInUrl=warn clone https://username:password@localhost attempt2 2>err &&
> +	test_must_fail git -c transfer.credentialsInUrl=warn clone https://username:password@224.0.0.1 attempt2 2>err &&
>  	grep "warning: $message" err >warnings &&
>  	test_line_count = 2 warnings &&
>  
> -	test_must_fail git -c transfer.credentialsInUrl=die clone https://username:password@localhost attempt3 2>err &&
> +	test_must_fail git -c transfer.credentialsInUrl=die clone https://username:password@224.0.0.1 attempt3 2>err &&
>  	grep "fatal: $message" err >warnings &&
>  	test_line_count = 1 warnings &&
>  
> -	test_must_fail git -c transfer.credentialsInUrl=die clone https://username:@localhost attempt3 2>err &&
> +	test_must_fail git -c transfer.credentialsInUrl=die clone https://username:@224.0.0.1 attempt3 2>err &&
>  	grep "fatal: $message" err >warnings &&
>  	test_line_count = 1 warnings
>  '

For this one one at least, it eventually gets around to setting up an actual httpd server with cloning etc. from $HTTPD_URL.

Can't we just do that for both of these tests rather than the the 224.0.0.0 hack? I.e. the root cause is that we're cleverly faking a not-a-server here, and now we're going to add another somewhat clever hack on top.

but since the test coverage is for https:// anyway, and we have other https tests against an actual server...

Previous: Johannes Schindelin via GitGitGadgetNext: Jeff King
Message 3 of 45 in “t5516/t5601: avoid using localhost for failing HTTPS requests”
  1. 0/2 t5516/t5601: avoid using localhost for failing HTTPS requestsJohannes Schindelin via GitGitGadget, Oct 31, 2022
  2. 1/2 t5516/t5601: avoid using `localhost` for failing HTTPS requestsJohannes Schindelin via GitGitGadget, Oct 31, 2022
  3. Ævar Arnfjörð BjarmasonOct 31, 2022
  4. Jeff KingOct 31, 2022
  5. Taylor BlauNov 1, 2022
  6. Jeff KingNov 1, 2022
  7. Jeff KingNov 1, 2022
  8. Jeff KingNov 1, 2022
  9. 1/2 t5516: move plaintext-password tests from t5601 and t5516Jeff King, Nov 1, 2022
  10. Ævar Arnfjörð BjarmasonNov 1, 2022
  11. Jeff KingNov 1, 2022
  12. Taylor BlauNov 1, 2022
  13. 2/2 t5516/t5601: be less strict about the number of credential warningsJeff King, Nov 1, 2022
  14. Ævar Arnfjörð BjarmasonNov 1, 2022
  15. Jeff KingNov 1, 2022
  16. Ævar Arnfjörð BjarmasonNov 1, 2022
  17. Jeff KingNov 1, 2022
  18. Ævar Arnfjörð BjarmasonNov 1, 2022
  19. Junio C HamanoNov 1, 2022
  20. Jeff KingNov 1, 2022
  21. Taylor BlauNov 1, 2022
  22. 2/2 t5516/t5601: be less strict about the number of credential warningsJohannes Schindelin via GitGitGadget, Oct 31, 2022
  23. Jeff KingOct 31, 2022
  24. Taylor BlauNov 1, 2022
  25. Jeff KingNov 1, 2022
  26. fetch: stop emitting duplicate transfer.credentialsInUrl=warn warningsÆvar Arnfjörð Bjarmason, Oct 31, 2022
  27. Taylor BlauNov 1, 2022
  28. Jeff KingNov 1, 2022
  29. Ævar Arnfjörð BjarmasonNov 1, 2022
  30. Taylor BlauNov 1, 2022
  31. Ævar Arnfjörð BjarmasonNov 1, 2022
  32. Taylor BlauNov 2, 2022
  33. 2/2 t5551: be less strict about the number of credential warningsJeff King, Nov 2, 2022
  34. Eric SunshineNov 2, 2022
  35. Jeff KingNov 2, 2022
  36. Eric SunshineNov 2, 2022
  37. Jeff KingNov 2, 2022
  38. Taylor BlauNov 3, 2022
  39. Jeff KingNov 1, 2022
  40. Ævar Arnfjörð BjarmasonNov 1, 2022
  41. Taylor BlauNov 1, 2022
  42. Ævar Arnfjörð BjarmasonNov 1, 2022
  43. Jeff KingNov 2, 2022
  44. Ævar Arnfjörð BjarmasonNov 4, 2022
  45. Jeff KingNov 4, 2022

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.