Re: [PATCH v4 01/11] add fetch-pack --diag-url tests for some corner cases

From: Torsten Bögershausen <tboegi@web.de>
Date: May 3, 2016, 16:30 UTC
Message-ID: <f9da0d57-2d85-6a1c-5a28-13956102f37b@web.de>
In-Reply-To: <xmqqzis7nn02.fsf@gitster.mtv.corp.google.com>

On 2016-05-03 18.07, Junio C Hamano wrote:

Show 20 quoted lines

> Mike Hommey <mh@glandium.org> writes:
> 
>> t5603-clone-dirname uses url patterns that are not tested with
>> fetch-pack --diag-url, and it would be useful if they were.
>>
>> Interestingly, some of those tests, involving both a port and a
>> user:password pair, don't currently pass. Note that even if a
>> user:password pair is actually not supported by git, the values used
>> could be valid user names (user names can actually contain colons
>> and at signs), and are still worth testing the url parser for.
> 
> I am not sure about the last part of this (and the tests in the
> patch for them).  When you are constrained by the Common Internet
> Scheme Syntax, i.e.
> 
>     <scheme>://<user>:<password>@<host>:<port>/<url-path>
> 
> you cannot have arbitrary characters in these parts; within the user
> and password field, any ":", "@", or "/" must be encoded.
>

I thinnk we have an old bug here:
	if (is_url(url_orig))
		url = url_decode(url_orig);
	else
		url = xstrdup(url_orig);

The the url should be separated into the components first, and afther that url-path should got into url_decode, and may be password, username.... (That's out of my head)