Re: [PATCH v2 4/5] bundle-uri: add support for http(s):// and file://

[Junio C Hamano <gitster@pobox.com>]

Derrick Stolee <derrickstolee@github.com> writes:

>>> +	if (istarts_with(uri, "https:") ||
>>> +	    istarts_with(uri, "http:"))
>> 
>> Let's be a bit more strict to avoid mistakes and make the code
>> immediately obvious, e.g.
>> 
>> 	if (istarts_with(uri, "https://") ||
>> 	    istarts_with(uri, "http://"))

This part (i.e. check for "<scheme>://", not "<scheme>:") still
stands, as the latter could be an scp style Git URL that goes to the
host whose name is <scheme>.  As mentioned later, s/istarts/starts/
is probably a good thing to do here.

>> Does "git-remote-https" talk to a "http://" URL just fine when uri
>> parameter starts with "http://"?  Would it be the same if the uri
>> parameter begins with say "Http://"?
>
> I did a quick check of our HTTPS tests modifying the HTTPD_PROTO
> variable in lib-httpd.sh to "HtTP" and we get this fun error:
>
> + git clone --filter=blob:limit=0 HtTP://127.0.0.1:5601/smart/server client
> Cloning into 'client'...
> git: 'remote-HtTP' is not a git command. See 'git --help'.
>
> So I guess I can keep case-sensitive comparisons here.

Guarding them to lowercase-only may sound like a cop-out to purists,
but I think it is reasonable thing to do.  The only folks that would
be offended by are protocol lawyers, and as your check shows, we are
treating <scheme> case sensitively already.

An obvious alternative is to downcase the "<scheme>://" part but I
do not think it is worth it; we have to do that everywhere and we
need to be confident that we covered all the code paths---the latter
is expensive.

There do exist skip_iprefix() that are used in many places, like
convert, http, mailinfo, etc. by the way.  That is a moot point as
we are not doing case insensitive comparison anymore.

Thanks.