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

From: Jeff King <peff@peff.net>
To: Taylor Blau <me@ttaylorr.com>
Cc: Johannes Schindelin via GitGitGadget <gitgitgadget@gmail.com>,
	git@vger.kernel.org, Derrick Stolee <derrickstolee@github.com>,
	Johannes Schindelin <johannes.schindelin@gmx.de>
Subject: Re: [PATCH 1/2] t5516/t5601: avoid using `localhost` for failing HTTPS requests
Date: Mon, 31 Oct 2022 22:28:19 -0400	[thread overview]
Message-ID: <Y2CEQwzz+nGR7PLf@coredump.intra.peff.net> (raw)
In-Reply-To: <Y2BvaXN2C325YUsz@nand.local>

On Mon, Oct 31, 2022 at 08:59:21PM -0400, Taylor Blau wrote:

> > >  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.

I don't think it was too difficult to move them. I just sent in patches
(which I just realized you were not cc'd on).

-Peff