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

[Taylor Blau <me@ttaylorr.com>]

On Mon, Oct 31, 2022 at 07:20:11PM -0400, Jeff King wrote:
> > 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).
>
> This feels pretty magical. I think it would be pretty unlikely for it to
> have a web server, but I wouldn't be surprised if there are systems
> where we get similar IP-routing hangs.
>
> Is there a reason not to move all of these tests into t5550 or t5551,
> where we have a real http server? That would be less magical, and then
> this first test:
>
> >  test_expect_success LIBCURL 'fetch warns or fails when using username:password' '
> > -	message="URL '\''https://username:<redacted>@localhost/'\'' uses plaintext credentials" &&
> > -	test_must_fail git -c transfer.credentialsInUrl=allow fetch https://username:password@localhost 2>err &&
> > +	message="URL '\''https://username:<redacted>@224.0.0.1/'\'' uses plaintext credentials" &&
> > +	test_must_fail git -c transfer.credentialsInUrl=allow fetch https://username:password@224.0.0.1 2>err &&
> >  	! grep "$message" err &&
>
> could be more robust. It would actually check that we succeeded in using
> the URL.

It is magical, indeed. If it's significantly more difficult to move
these into t5550 or t5551, then I'm OK with this in the meantime (since
GitHub Actions really is our primary CI target that we care about this
not hanging on).

But assuming that moving these around isn't that difficult, I would be
slightly happier to see these tests in one of the aforementioned spots.

Thanks,
Taylor