From: Junio C Hamano Date: Thu, 04 Aug 2022 18:19:59 GMT Subject: Re: [PATCH v2 4/5] bundle-uri: add support for http(s):// and file:// Message-ID: In-Reply-To: <9afd5eb2-44a0-6342-6006-5dbdefba9947@github.com> Derrick Stolee 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 "://", not ":") still stands, as the latter could be an scp style Git URL that goes to the host whose name is . 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 case sensitively already. An obvious alternative is to downcase the "://" 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.